-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
DEP: deprecate np.finfo(None) #23011
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
Conversation
numpy/core/getlimits.py
Outdated
if dtype is None: | ||
warnings.warn( | ||
"dtype cannot be None. This behavior " | ||
"will raise an error in the future.", |
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.
Thanks, the warning is indeed the correct path. Could you add a test as well? It can live in test_deprecations.py
.
Please add (Deprecated in NumPy 1.25)
or something similar to the message. Could also lead with finfo() dtype ...
just to make it even easier to find where it is coming from.
The last thing is that both here and in the test, we try to add a comment:
# Deprecated in NumPy 1.25, 2022-01-16
Or similar. This makes it much easier to find and to act on in the future.
Thanks, this looks good! I forgot, but since it is a deprecation, would you be up for also adding a brief release note? There is an explanation on how in |
Ver minimal :). Thanks for all the follow-ups @dshintani404. |
CircleCI is a bit confused right and mostly seem to have run through even, should be fine. @tupui about the circleCI failure here: CircleCI is failing here due to the SciPy incompatibility on current main. The point being: It wasn't failing on main, it seems even though plotting breaks the "with warnings" run ignoring the error result hides issues bigger then the warnings? |
@dshintani404 if you feel like digging into C and much more broadly. I don't mind that this zoomed in on |
Super strange, you might be able to temporary use a nitpick exception until you have a proper fix. |
@tupui to be clear, what makes me pause is that I think it should be failing CI currently on all PRs, not that it failed here. I.e. see here: (around line 180 of the "build devdocs" step). There is an exception building, but CI is green. |
Yes this is what we said in the other PR, because there is a OR, it's always returning green. My understanding from what Matti said, was that it is fine as you need to really have a look either way. |
I think I did not realize what I was agreeing to. Previously, we had a non-nitpick build that would fail properly when the build did not successfully complete, and an nitpick build that would always succeed but would print the number of warnings emitted. Now that we have compbined them, we do not have a way of indicating that there is a failure generating documentation, and not just a failure to format the generated documentation properly. |
Yeah, I expected that a "critical" failure would float up but admit that is my error. So, not sure that this won't hide too many issues. |
Let me know if I should add the second build back. One other possibility would be to not clean before rebuilding. This would be faster and still break if there is really an issue with the doc no? |
Yeah, maybe lets do that, that would be great! I guess you are right about the not-cleaning, although I am not sure it actually speeds things up much. |
Hi-five on merging your first pull request to NumPy, @dshintani404! We hope you stick around! Your choices aren’t limited to writing code – you can review pull requests, help us stay on top of new and old issues, develop educational material, work on our website, add or improve graphic design, create marketing materials, translate website content, write grant proposals, and help with other fundraising initiatives. For more info, check out: https://numpy.org/contribute |
Thank you for your help! |
I read relevant codes in C and understood that a point to fix was probably around dtypemeta.c, but now I am completely stacked. |
Thee relevant code is in |
See #14684