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

Jump to bottom

BUG: stats.binned_statistic_2d user function expecting arrays #13633

Merged
merged 1 commit into from Mar 13, 2021
Merged

BUG: stats.binned_statistic_2d user function expecting arrays #13633

merged 1 commit into from Mar 13, 2021

Conversation

tupui
Copy link
Member

@tupui tupui commented Mar 1, 2021

The documentation states:

function : a user-defined function which takes a 1D array of values, and outputs a single numerical statistic. This function will be called on the values in each bin. Empty bins will be represented by function([]), or NaN if this returns an error.

As shown in gh-13608, users can expect in their functions a numpy array.

I propose to not change the doc and just give a numpy array to the function.

Closes gh-13608

@tupui tupui added maintenance Items related to regular maintenance tasks scipy.stats labels Mar 1, 2021
@tupui tupui modified the milestone: 1.7.0 Mar 8, 2021
@tupui
Copy link
Member Author

tupui commented Mar 8, 2021

Sorry wrong issue.

@tupui
Copy link
Member Author

tupui commented Mar 8, 2021 •
edited

@rgommers I have these results on my local machine:

       before           after         ratio
     [8ce88218]       [869c8695]
     <master>         <maint_binned_stats_func>
-      56.0±0.6ms         52.2±1ms     0.93  stats.BinnedStatisticDD.time_binned_statistic_dd(<function std at 0x7f8e8cd758b0>)
-      56.2±0.5ms       52.2±0.7ms     0.93  stats.BinnedStatisticDD.time_binned_statistic_dd_reuse_bin(<function std at 0x7f8e8cd758b0>)
-      16.7±0.4ms       15.2±0.2ms     0.91  stats.BinnedStatisticDD.time_binned_statistic_dd('max')
-      17.1±0.5ms       15.0±0.2ms     0.87  stats.BinnedStatisticDD.time_binned_statistic_dd('min')

@rgommers
Copy link
Member

Ah, it's a little faster - nice.

rgommers
rgommers approved these changes Mar 13, 2021
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM, merging. Thanks @tupui.

For the record, I think this is a bug, not a doc issue. If there were users that relied on the passed values being a list and not an array (which would be very strange), they're out of luck.

@rgommers rgommers merged commit b0936fe into scipy:master Mar 13, 2021
@rgommers rgommers added this to the 1.7.0 milestone Mar 13, 2021
@tupui tupui deleted the maint_binned_stats_func branch March 13, 2021 20:57
@tupui
Copy link
Member Author

tupui commented Mar 13, 2021

Thanks @rgommers!

@tupui tupui changed the title MAINT: stats.binned_statistic_2d user function expecting arrays BUG: stats.binned_statistic_2d user function expecting arrays Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgommers rgommers rgommers approved these changes

@tylerjereddy tylerjereddy tylerjereddy left review comments

Assignees
No one assigned
Labels
maintenance Items related to regular maintenance tasks scipy.stats
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.

Why does stats.binned_statistic_2d convert its values argument from array into a list?
3 participants
@tupui @rgommers @tylerjereddy