Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weighted histograms #764

Merged
merged 5 commits into from
Oct 7, 2023
Merged

Weighted histograms #764

merged 5 commits into from
Oct 7, 2023

Conversation

matteobachetti
Copy link
Member

Allow our fast histograms to accept weights. Also, uniformize the interface so that we have the same options across all 1d, 2d, 3d, and nd histograms. Use a single function to fail safe to the numpy implementation of each kind of histogram when Numba throws a typing error.
Breaking change: the option for data range is now range like in Numpy. It made no sense to have a different name for the different implementations.

@codecov
Copy link

codecov bot commented Oct 1, 2023

Codecov Report

Merging #764 (5d3c2e1) into main (3fc3c15) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 5d3c2e1 differs from pull request most recent head 4f55b87. Consider uploading reports for the commit 4f55b87 to get more accurate results

@@            Coverage Diff             @@
##             main     #764      +/-   ##
==========================================
+ Coverage   96.14%   96.15%   +0.01%     
==========================================
  Files          43       43              
  Lines        7937     7966      +29     
==========================================
+ Hits         7631     7660      +29     
  Misses        306      306              
Files Coverage Δ
stingray/utils.py 98.91% <100.00%> (+0.05%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@mgullik mgullik left a comment

Choose a reason for hiding this comment

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

Hi @matteobachetti,

I'm not an expert on this topic, so please excuse me if my comments are not correct.
I noticed that for the function hist1d_numba_seq_weight you start immediately with an example while for the other histogram definitions (e.g. histogramnd) you wrote a couple of lines of explanation before the example. I think this is very useful, and it would be useful even in hist1d_numba_seq_weight.

I know that a histogram object is pretty self-explanatory, but does it make sense to write a few more lines explaining what is the difference between histogram and histogramdd, for people that are not super familiar with the object of numpy?

@matteobachetti matteobachetti added this pull request to the merge queue Oct 7, 2023
Merged via the queue into main with commit 17fbaf0 Oct 7, 2023
5 checks passed
@matteobachetti matteobachetti deleted the weighted_histograms branch October 7, 2023 07:38
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.

2 participants