-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix NumPy deprecation of setting np.int16
with out-of-bound value
#13835
Conversation
👋 Thank you for your draft pull request! Do you know that you can use |
astropy/io/fits/tests/test_core.py
Outdated
n[0] = 1 | ||
n[1] = 60000 | ||
n[2] = 2 | ||
n = np.array([1, 60000, 0]).astype('i2') |
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 think it has been pure luck that this test worked -- that it wrapped around automatically. I think we should just be explicit and write n[1] = 60000 - 65536
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 by construction fits.Column
is setting it with a int16 that is declared to be interpreted as uint16, so would make sense to create it this way. Or perhaps a bit more explicitly
np.array([1, 60000, 0], dtype=np.uint16).astype(np.int16)
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.
OK, that's true. Maybe use your more explicit route?
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.
Don't want to hold up this nice fix for no reason, so let me approve. If you have the oomph to add the dtype='u2'
, then please do, otherwise also fine to just merge.
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!
Thanks! |
…` with out-of-bound value
…` with out-of-bound value
…835-on-v5.1.x Backport PR #13835 on branch v5.1.x (Fix NumPy deprecation of setting `np.int16` with out-of-bound value)
…835-on-v5.0.x Backport PR #13835 on branch v5.0.x (Fix NumPy deprecation of setting `np.int16` with out-of-bound value)
Description
This pull request is to address deprecation in NumPy 1.24 of implicit casting of np.int types with out-of-bond numbers (thus forcing an overflow); replacing this with an explicit
np.array(a).astype(np.int16)
Fixes #13834
Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
Extra CI
label. Codestyle issues can be fixed by the bot.no-changelog-entry-needed
label. If this is a manual backport, use theskip-changelog-checks
label unless special changelog handling is necessary.astropy-bot
check might be missing; do not let the green checkmark fool you.backport-X.Y.x
label(s) before merge.