Update yt for Numpy 1.13 #1422
Merged
Update yt for Numpy 1.13 #1422
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is a pretty cool thing to do -- thank you for taking this on, @ngoldbaum ! |
|
I will try to look this over this week. |
jzuhone
approved these changes
Jun 4, 2017
lindsayad
reviewed
Jun 9, 2017
| # numpy 1.13 or newer | ||
| from numpy import positive, divmod as divmod_, isnat, heaviside | ||
| except ImportError: | ||
| positive, divmod_, isnat, heaviside = (None,)*4 |
There was a problem hiding this comment.
No, I'm setting these symbols to None since they're not available in older versions of Numpy
There was a problem hiding this comment.
Ah I see now. Figured I was not seeing something :-)
There was a problem hiding this comment.
One plus of me reviewing PRs is that I learn something new about python (even seemingly trivial things) every time
ngoldbaum
commented
Jun 9, 2017
yt/units/yt_array.py
Outdated
| >>> np.ones_like(a) | ||
| YTArray([1, 1, 1, 1, 1, 1, 1, 1]) g/cm**3 | ||
| >>> np.abs(a) | ||
| YTArray([8, 8, 8, 8, 8, 8, 8, 8]) g/cm**3 |
Xarthisius
approved these changes
Jun 15, 2017
matthewturk
added a commit
to matthewturk/yt
that referenced
this pull request
Apr 17, 2018
Update yt for Numpy 1.13
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates yt for changes in Numpy 1.13.
Note
Github isn't rendering the diff for yt_array.py, you need to manually expand it if you want to see the bulk of the changes in this PR.
You may be surprised by the size of the changes here - this is mostly to support a new feature in Numpy 1.13:
__array_ufunc__. See this page for more details.Briefly, defining
__array_ufunc__allows us to skip much of the boilerplate in the definition ofYTArray, since almost all array operations will now be handled by__array_ufunc__. This reduces overhead for many operations and also makes the overall implementation simpler.Unfortunately, we still need to support older numpy releases. I've decided that the best way to do this is to make a large chunk of the body of
YTArraydefined conditionally on numpy version. This means that this PR has a large net increase in code length, since__array_wrap__and__array_ufunc__have similar implementations. I've tried my best to refactor completely identical code in these two functions into private helper functions.You'll also see in the tests that the answers for a limited subset of the
yt.unitstests now depend on numpy version. In my opinion these are all beneficial changes, but if anyone has questions about any specific change let me know. The most significant change is that yt will no longer raiseYTUfuncUnitError. That means it's now possible to do something like:This will not work correctly on older NumPy versions, so we won't be able to take advantage of this improvement in yt's internals for a few years.