-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
BUG: weighted nanpercentile, nanquantile and multi-dim q #26582
Conversation
Linter failures are real. I unfortunately don't have time to understand the code to give you a proper code review right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand all the black magic tricks of the slicing/ndindexing expressions but the test looks good.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
I pushed some changes, to simplify the logic with moveaxes and add tests for multi-dimensional inputs. The problem if you add tests... is that you find the other bugs in the old code paths and need to fix them too. Multi-dimensional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But someone else should have a quick look over...
q
q
Thanks @lorentzenchr and reviewers. |
* TST add test_nan_value_with_weight * FIX weighted nanquantile * CLN appease linter * Update numpy/lib/tests/test_nanfunctions.py Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> * Simplify code, expand old and add new ndim test with weights * BUG: Expand test and fix multi-dimensional q in the normal path... --------- Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net> Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Sebastian Berg <sebastianb@nvidia.com>
* TST add test_nan_value_with_weight * FIX weighted nanquantile * CLN appease linter * Update numpy/lib/tests/test_nanfunctions.py Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> * Simplify code, expand old and add new ndim test with weights * BUG: Expand test and fix multi-dimensional q in the normal path... --------- Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net> Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Sebastian Berg <sebastianb@nvidia.com>
Fixes #26455.
Hint: The solution is no beauty, but it does seem to do it's job. The whole
nanpercentile
seems convoluted given that it could rely more onpercentile
.