fix: change default reduce_C_function to np.sum for hexbin
#408
Conversation
Updated default behavior for weights/C to compute total instead of average.
beckermr
changed the title
reduce_C_function to np.sum for hexbin
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Can you add a test for this to run over the bin plots and ensure we have the same behavior? |
cvanelteren
left a comment
cvanelteren
left a comment
There was a problem hiding this comment.
We need to add a test that ensures we are performing the same operations across the binned plots
|
Any suggestions for how to write the test? |
|
I think we can do a simple check where take all hist related functions and check that the patches contain the sum; doesn't have to be super elaborate but just a safeguard for if this changes on mpl or our end in the future. We could also check if what function is called with patches to see what the function put as input and check that this is a sum to ensure we are consistent across the board. We may also need some smoke tests, like does passing a cumstom func properly propagate and set the values accordingly. This would not necessarily need a visual check. |
|
ok @cvanelteren this is ready for another look! |
|
Great thanks! |
This PR fixes some unexpected behavior w/
hexbin.hexbinparameterChas been mapped to the parameterweightsin ultraplot.weightsare summed for the points in each bin (along with possibly a global normalization correction).hexbinis to average theweightsin each bin.This PR changes the default for
hexbinto sum theweightsin each bin so that the semantics of howweightsare used is consistent across plotting functions.