ENH: remove "Invalid value" warnings from median, percentile #12679
Conversation
|
Are there any places where we do want this warning? It would be good to know what the motivation for it was in the first place - in all cases, it seems redundant vs just checking for Nan in the output. |
|
This PR is the original source: #5753 It added these warnings at the same time as it added a guarantee that NaN is returned if any NaNs are encountered. The release notes do say: "Similar to mean, median and percentile now emits a Runtime warning and But I'm not sure it was ever true that mean would emit a runtime warning when NaNs are encountered. Possibly we used to be more aggressive about warning when an operation that already includes NaNs also returns in an NaN value? |
|
Are there any tests for things such as |
|
|
||
| def test_empty(self): | ||
| # empty arrays | ||
| # mean(empty array) emits two warnings: empty slice and divide by 0 |
There was a problem hiding this comment.
@seberg this test uses an empty array
There was a problem hiding this comment.
Sorry, nvm. had thought about it a bit earlier and forgot again ;). All fine for sure length 0 would still have a division by zero. And things such as +inf - inf already warn during the sum step I would assume at least.
|
@shoyer That implies that at least @madphysicist is/was relying on the current behavior. Hmm... |
|
Rereading that message, I think it was an oversight from us at the time that we decided to go with the warning (maybe to be on the safe side being unsure). I believe what the message is really about is to return NaN at all. So I do not think anyone relies on it and we can merge this. Thanks Matti! |
|
Just to note. I checked the |
Fixes #12676 by removing the
Invalid valuewarning frommedian,percentileandquantile