Fix non-tuple NumPy indexing in MetPy #910
Merged
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
Since NumPy 1.15 came out, we have been getting pages and pages of warnings about our use of non-tuple based indexing. This commit converts all of our previous list-based indexing to tuple-based indexing. This doesn't yet clear out all the warnings (some of them are upstream in Pint and scipy), but it does drop the number down from 600-something to 10-something.
jthielen
added
Type: Maintenance
dopplershift
reviewed
Aug 1, 2018
| left = (- big_delta / (combined_delta * delta[delta_slice0]) * f[slice0] + | ||
| combined_delta / (delta[delta_slice0] * delta[delta_slice1]) * f[slice1] - | ||
| delta[delta_slice0] / (combined_delta * delta[delta_slice1]) * f[slice2]) | ||
| combined_delta = delta[tuple(delta_slice0)] + delta[tuple(delta_slice1)] |
There was a problem hiding this comment.
Ugh...this is ugly enough for me to want to make something to hide this absurdity.
There was a problem hiding this comment.
Would a pair of _convert_to_tuple(*args) and _convert_to_list(*args) helpers be a good approach? Or do you have an idea for another approach?
There was a problem hiding this comment.
I'm thinking something more OO, and possibly more automatic. I don't have anything particular in mind at the moment, but need to play with some ideas.
|
Any improvements to clean up the indexing code can be discussed or reference #912. |
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.
Since NumPy 1.15 came out, we have been getting pages and pages of warnings about our use of non-tuple based indexing. This PR converts all of our previous list-based indexing to tuple-based indexing. This doesn't yet clear out all the warnings (some of them are upstream in Pint and scipy), but it does drop the number down from 600-something to 10-something.
Closes #900.