-
-
Notifications
You must be signed in to change notification settings - Fork 11.9k
MAINT: Improve error handling in umathmodule setup #17014
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
33a504e to
318a4e7
Compare
numpy/core/src/umath/umathmodule.c
Outdated
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.
This list had gotten very stale - seemed better to avoid it altogether
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.
True, the conditional style is not really used in NumPy, but I agree with using it here, everything else is just extremely verbose.
Really this only ensures MemoryErrors propagate, nothing else can go wrong here.
318a4e7 to
47bda4c
Compare
seberg
left a comment
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 Eric. LGTM modulo missing (unimportant) DECREF and test success.
numpy/core/src/umath/umathmodule.c
Outdated
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.
True, the conditional style is not really used in NumPy, but I agree with using it here, everything else is just extremely verbose.
|
Thanks Eric, I took the liberty of just adding the DECREF. |
Really this only ensures MemoryErrors propagate, nothing else can go wrong here.
Addresses item 3 from #17012. I don't think this is worth considering a bug, since running out of memory when creating a ufunc is not likely.