Raise error if user slices NDCube with Ellipsis. #224
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
|
Thanks for the pull request @DanRyanIrish! Everything looks great! |
DanRyanIrish
force-pushed
the
issue216
branch
from
bf9e12a to
dce8a38
Compare
|
Getting an |
Cadair
reviewed
Dec 16, 2019
ndcube/utils/wcs.py
Outdated
| @@ -183,6 +183,10 @@ def _wcs_slicer(wcs, missing_axes, item): | |||
| new_wcs = wcs.slice(item_checked) | |||
| # if it a tuple like [0:2, 0:3, 2] or [0:2, 1:3] | |||
| elif isinstance(item, tuple): | |||
| # Ellipsis slicing is currently not supported. | |||
| # Raise an error if user tries to slice by ellipsis. | |||
| if type(Ellipsis) in [type(item_i) for item_i in item]: | |||
There was a problem hiding this comment.
Can't you do this with:
Suggested change
| if type(Ellipsis) in [type(item_i) for item_i in item]: | |
| if Ellipsis in item: |
?
DanRyanIrish
force-pushed
the
issue216
branch
from
dce8a38 to
de06195
Compare
|
You should probably add a regression test 😀 |
|
A regression test?!?!?! |
|
yeah so we don't break this later 😛 |
|
@Cadair I don't understand the difference between a regression test and just running all the existing tests... |
|
Apart from a test, is there anything blocking you from approving this PR? |
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.
Description
ndcube 1.x does not support slicing
NDCubes withEllipses. However, when a user tries this, the operation does not fail, but rather slices the data correctly, but the FITS-WCS incorrectly. This PR causes the operation to fail with an informative error.Fixes #216