Make approx more compatible with numpy #2606
Merged
Conversation
This file contains hidden or 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
Previously I was subverting the natural order of operations by subclassing from `ndarray`, but it turns out that you can tell just numpy to call your operator instead of its by setting the `__array_priority__` attribute on your class. This is much simpler, and it turns out the be a little more robust, too.
I'm not sure why they were removed...
nicoddemus
approved these changes
Jul 22, 2017
Member
nicoddemus
left a comment
nicoddemus
left a comment
There was a problem hiding this comment.
@kalekundert awesome, thanks a lot for this!
Member
It does, I executed |
Member
|
Thanks again @kalekundert! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
I think this fixes an issue that came up in #2599 -- where certain
approxcomparisons with numpy arrays trigger a DeprecationWarning -- by simplifying how approx interfaces with numpy arrays.Previously, approx used inheritance to guarantee that it's
__eq__operator was called rather than the numpy array's. This worked most of the time, but the DeprecationWarning in #2599 was due to a shortcoming of this approach. (It think it also would've choked on comparisons with subclasses ofnp.ndarray, but I never tested this.)It turns out that there's a better way to make sure that
approx.__eq__gets called, and that's to setapprox.__array_priority__to a large number. Whenever anything is compared to an array, numpy checks for this attribute and gives priority to whichever object has a higher value. This should avoid the DeprecationWarning, and just in general be more robust. It's also a lot simpler, so I was able to cut out all of the complex dynamic inheritance code that was in there before.This PR also adds the numpy tests back into tox. My bisect-fu failed me and I couldn't really figure out why they were removed in the first place, but I suspect it was by mistake.