Implements newaxis #35
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
scopatz
reviewed
Jun 4, 2020
ndindex/slice.py
Outdated
| >>> idx.newshape((8, 10), axis=1) | ||
| (8, 6) | ||
| """ | ||
| from . import Tuple |
There was a problem hiding this comment.
You can try it but it might fail due to circular imports.
scopatz
reviewed
Jun 4, 2020
|
Code looks good to me |
|
@asmeurer - please review and merge |
asmeurer
reviewed
Jun 15, 2020
ndindex/ndindex.py
Outdated
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| from numpy import ndarray | |||
|
|
|||
| newaxis = None | |||
There was a problem hiding this comment.
This needs to be implemented as an NDIndex subclass.
There was a problem hiding this comment.
OK, I just went with that NumPy does. Let me know if this is what you had in mind
Also: - Add Newaxis.reduce() to the See Also of other reduce() docstrings. - Do shape checking in Newaxis.reduce(). - Move raw up to the top of the class definition.
Also adds newaxes to the Tuple and ndindices hypothesis strategies, and updates the ndindices hypothesis strategy to use filter instead of composite (which makes it just ndindices instead of ndindices()).
|
I've pushed updates here. This is ready for review. |
|
Yes, getting the logic for newaxis right in the various Tuple methods was not easy. However, I've run the hypothesis test suite with lots of examples and no failures, so I'm fairly confident the code here is correct. |
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.
Closes #24