Support Ellipsis in NDCube.__getitem__ #818
Conversation
|
This will need some unit tests and a changelog entry. |
|
there is no file named test_ndslicing.py . Should I make one and add test cases there or use test_ndcube.py where some slicing tests are present. |
|
I see no harm in creating a new dedicated slicing test file. |
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
Should I add init.py to tests folder as error raised in pre-commit.ci |
|
Is there not an init file in that folder? If not, one needs to be added. |
| @pytest.fixture | ||
| def sample_ndcube(): | ||
| data = np.random.rand(4, 4, 4) | ||
| wcs = WCS(naxis=3) | ||
| return NDCube(data, wcs) |
There was a problem hiding this comment.
I thought there were other fixtures defined elsewhere which can be used for the test below.
I created tests folder in mixins. All tests folder have init.py file which is empty. I think I should add one. |
|
Sounds good |
|
any suggestions on the changes? |
|
I think there should be a few more tests on more complex ndcubes. But I will ping the maintainers for a review |
There was a problem hiding this comment.
Hi @Diya910. Thanks for this really helpful PR. I agree with @nabobalis that there should be a few more tests, though. I've left a code suggestion outlining some additional parameterized tests.
Also, there are slicing tests in test_ndcube.py. So the tests for this PR should probably go there too.
Finally, this branch needs to be updated with main.
Thanks again for the PR!
| if isinstance(item, tuple) and Ellipsis in item: | ||
| if item.count(Ellipsis) > 1: | ||
| raise IndexError("An index can only have a single ellipsis ('...')") | ||
|
|
| expanded_item.extend([slice(None)] * (len(self.shape) - len(item) + 1)) | ||
| else: | ||
| expanded_item.append(i) | ||
|
|
| def test_ellipsis_usage(sample_ndcube): | ||
| sliced_cube = sample_ndcube[..., 1] | ||
| assert sliced_cube.data.shape == (5,10,12) |
There was a problem hiding this comment.
| def test_ellipsis_usage(sample_ndcube): | |
| sliced_cube = sample_ndcube[..., 1] | |
| assert sliced_cube.data.shape == (5,10,12) | |
| @pytest.mark.parametrize(("ndc","item","expected_shape") | |
| [ | |
| ("ndcube_4d_ln_l_t_lt", np.s_[..., 1], (5, 10, 12)), | |
| ("ndcube_4d_ln_l_t_lt", np.s_[..., 1:, 1], (5, 10, 11)), | |
| ("ndcube_4d_ln_l_t_lt", np.s_[1, ...], (10, 12, 8)), | |
| ("ndcube_4d_ln_l_t_lt", np.s_[1, 1:, ...], (9, 12, 8)), | |
| ("ndcube_4d_ln_l_t_lt", np.s_[1, ..., 1:], (10, 12, 7)), | |
| ("ndcube_4d_ln_l_t_lt", np.s_[1, 1:, ..., 1:], (9, 12, 7)), | |
| ], | |
| indirect=("ndc",)) | |
| def test_ellipsis_usage(ndc): | |
| sliced_cube = ndc[item] | |
| assert sliced_cube.data.shape == expected_shape |
PR Description
This PR adds support for the ellipsis (...) syntax in NDCube allowing users to slice NDCube objects using ellipsis notation.
Issue Reference:#741