Skip to content

feat!: automatic bin number calculation for variable registry#232

Merged
0ctagon merged 11 commits into8-hat:mainfrom
CSantos01:main
Oct 1, 2025
Merged

feat!: automatic bin number calculation for variable registry#232
0ctagon merged 11 commits into8-hat:mainfrom
CSantos01:main

Conversation

@CSantos01
Copy link
Contributor

@CSantos01 CSantos01 commented Sep 29, 2025

Added the automatic bin number calculation for variable registry as suggested by the eponymous issue and updated the docs+examples.

The function work similarly to update_variable_registry_ranges and is called update_variable_registry_binning.

Attached are the comparisons between the previous default binning (50) and the automatic binning using numpy.histogram_bin_edges with option "auto" for the bins variable.

Test auto-binning with 100k events:

Test_binnings_variable_0.pdf
Test_binnings_variable_0

Test auto-binning with 1k event:

Test_binnings_variable_0_1k.pdf
Test_binnings_variable_0_1k

@0ctagon 0ctagon requested review from 0ctagon, Copilot and cyrraz and removed request for Copilot and cyrraz September 29, 2025 23:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds automatic bin number calculation functionality to the variable registry system. It introduces the update_variable_registry_binning function that calculates optimal bin numbers using numpy's "auto" binning algorithm instead of the default 50 bins.

  • Implements update_variable_registry_binning function using numpy's histogram_bin_edges with "auto" option
  • Updates documentation and examples to demonstrate the new binning functionality
  • Adds comprehensive test coverage for the new function including error cases and edge conditions

Reviewed Changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/plothist/variable_registry.py Adds the core update_variable_registry_binning function implementation
src/plothist/init.py Exports the new function in the public API
tests/test_variable_registry.py Comprehensive test suite for the new binning functionality
docs/basics/variable_registry.rst Documentation updates explaining the new binning feature
src/plothist/examples/2d_hist/2d_hist_correlations.py Example usage of the new binning function

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@0ctagon 0ctagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution!

@CSantos01
Copy link
Contributor Author

After discussion with @0ctagon and based on his comments, we decided to merge both update_variable_registry_ranges and update_variable_registry_binning into one, namely update_variable_registry_binning.

…r` in the `update_variable_registry_binning` function
@0ctagon 0ctagon requested a review from Copilot September 30, 2025 08:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 8 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@cyrraz cyrraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! I left a comment.

@cyrraz cyrraz requested a review from Copilot September 30, 2025 13:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 8 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@cyrraz cyrraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Please check my new comment.

Copy link
Member

@0ctagon 0ctagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the small comment. PR is good for me to be merged. Don't forget to add ! in the name of the squashed commit ;)

@0ctagon 0ctagon changed the title feat: automatic bin number calculation for variable registry feat!: automatic bin number calculation for variable registry Oct 1, 2025
@0ctagon 0ctagon merged commit d0d2802 into 8-hat:main Oct 1, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants