From 70fd7761e3c3d23867c43162641d5f74dc693fe7 Mon Sep 17 00:00:00 2001 From: Nabil Freij Date: Thu, 11 Jun 2026 14:34:28 -0700 Subject: [PATCH 1/5] Name expected world objects in crop count and type errors When a crop point has the wrong number of entries, or an entry of the wrong class, the error now lists the expected world objects in order, so users migrating from a WCS with fewer world objects, or passing objects in the wrong order, learn the fix from the error itself. --- ndcube/ndcube.py | 7 +++++-- ndcube/tests/test_ndcube_slice_and_crop.py | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/ndcube/ndcube.py b/ndcube/ndcube.py index 87ece2c04..e93b9b237 100644 --- a/ndcube/ndcube.py +++ b/ndcube/ndcube.py @@ -640,15 +640,18 @@ def _get_crop_item(self, *points, wcs=None, keepdims=False): if comp.count(c) > 1: comp.pop(k) classes = [wcs.world_axis_object_classes[c][0] for c in comp] + expected = ", ".join(f"{name} ({cls.__name__})" for name, cls in zip(comp, classes)) for i, point in enumerate(points): if len(point) != len(comp): raise ValueError(f"{len(point)} components in point {i} do not match " - f"WCS with {len(comp)} components.") + f"WCS with {len(comp)} components. Each point must " + "have one entry per world object (use None for a " + f"component that should not be cropped), in order: {expected}.") for j, value in enumerate(point): if not (value is None or isinstance(value, classes[j])): raise TypeError(f"{type(value)} of component {j} in point {i} is " f"incompatible with WCS component {comp[j]} " - f"{classes[j]}.") + f"{classes[j]}. Expected order: {expected}.") return utils.cube.get_crop_item_from_points(points, wcs, False, keepdims=keepdims, original_shape=self.data.shape) diff --git a/ndcube/tests/test_ndcube_slice_and_crop.py b/ndcube/tests/test_ndcube_slice_and_crop.py index 56b7cd4a4..02aee0566 100644 --- a/ndcube/tests/test_ndcube_slice_and_crop.py +++ b/ndcube/tests/test_ndcube_slice_and_crop.py @@ -287,7 +287,8 @@ def test_crop_missing_dimensions(ndcube_4d_ln_lt_l_t): interval0 = cube.wcs.array_index_to_world([1, 2], [0, 1], [0, 1], [0, 2])[0] lower_corner = [interval0[0], None] upper_corner = [interval0[-1], None] - with pytest.raises(ValueError, match=r'2 components in point 0 do not match WCS with 3'): + with pytest.raises(ValueError, match=r'2 components in point 0 do not match WCS with 3 .* in order: ' + r'time \(Time\), spectral \(Quantity\), celestial \(SkyCoord\)\.$'): cube.crop(lower_corner, upper_corner) @@ -299,7 +300,8 @@ def test_crop_mismatch_class(ndcube_4d_ln_lt_l_t): lower_corner = [coord[0] for coord in intervals] upper_corner = [coord[-1] for coord in intervals] with pytest.raises(TypeError, match=r" of component 0 in point 0 is " - r"incompatible with WCS component time"): + r"incompatible with WCS component time .* Expected order: " + r"time \(Time\), spectral \(Quantity\), celestial \(SkyCoord\)\.$"): cube.crop(lower_corner, upper_corner) From 30881a742de1e337cc07b3282bd073da844ae0c6 Mon Sep 17 00:00:00 2001 From: Nabil Freij Date: Sun, 27 Sep 2026 11:14:43 -0700 Subject: [PATCH 2/5] Fix crop world-object order for non-contiguous WCS components The component dedup in NDCube._get_crop_item popped entries while enumerating, which kept the last occurrence of each world object. WCSes whose object components are not adjacent (e.g. HPLN, WAVE, HPLT, or DKIST VISP's lon, wl, lat, time, stokes) then rejected points given in the order pixel_to_world returns, including partial points with None. Keep the first occurrence instead. As astropy's world_to_pixel does, also match objects to components by class when each object in a point matches exactly one class and no two objects match the same one (#608). Points in the old order therefore keep working when the world objects have distinct classes, none a subclass of another, as in dkist's VISP crop tests. Otherwise, for example when objects share a class or their classes are related by subclassing (Quantity and SpectralCoord), the whole point is matched by position. Also sort the pixel axes with input in get_crop_item_from_points: iterating a set swapped the bounds of pixel axes (e.g. 3 and 8) on cubes with nine or more dimensions. --- changelog/crop-object-order.bugfix.1.rst | 1 + changelog/crop-object-order.bugfix.rst | 1 + ndcube/ndcube.py | 13 +++++---- ndcube/tests/test_ndcube_slice_and_crop.py | 33 ++++++++++++++++++++++ ndcube/utils/cube.py | 2 +- 5 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 changelog/crop-object-order.bugfix.1.rst create mode 100644 changelog/crop-object-order.bugfix.rst diff --git a/changelog/crop-object-order.bugfix.1.rst b/changelog/crop-object-order.bugfix.1.rst new file mode 100644 index 000000000..1bbb0e022 --- /dev/null +++ b/changelog/crop-object-order.bugfix.1.rst @@ -0,0 +1 @@ +Fixed `~ndcube.NDCube.crop` and `~ndcube.NDCube.crop_by_values` swapping the bounds of some pixel axes on cubes with nine or more dimensions. diff --git a/changelog/crop-object-order.bugfix.rst b/changelog/crop-object-order.bugfix.rst new file mode 100644 index 000000000..5d92e9fbf --- /dev/null +++ b/changelog/crop-object-order.bugfix.rst @@ -0,0 +1 @@ +Fixed `~ndcube.NDCube.crop` rejecting points given in the order returned by ``pixel_to_world``, including partial points with ``None``, when a WCS's world objects are interleaved (e.g. ``HPLN, WAVE, HPLT``). Objects that each match exactly one world object class can now also be given in any order, as with `~astropy.wcs.wcsapi.BaseHighLevelWCS.world_to_pixel`, and the errors for a point of the wrong length or type list the expected objects in order. diff --git a/ndcube/ndcube.py b/ndcube/ndcube.py index e93b9b237..19c393c6f 100644 --- a/ndcube/ndcube.py +++ b/ndcube/ndcube.py @@ -633,12 +633,7 @@ def _get_crop_item(self, *points, wcs=None, keepdims=False): # Quit out early if we are no-op if no_op: return tuple([slice(None)] * wcs.pixel_n_dim) - comp = [c[0] for c in wcs.world_axis_object_components] - # Trim to unique component names - `np.unique(..., return_index=True) - # keeps sorting alphabetically, set() seems just nondeterministic. - for k, c in enumerate(comp): - if comp.count(c) > 1: - comp.pop(k) + comp = utils.misc.unique_sorted(c[0] for c in wcs.world_axis_object_components) classes = [wcs.world_axis_object_classes[c][0] for c in comp] expected = ", ".join(f"{name} ({cls.__name__})" for name, cls in zip(comp, classes)) for i, point in enumerate(points): @@ -647,6 +642,12 @@ def _get_crop_item(self, *points, wcs=None, keepdims=False): f"WCS with {len(comp)} components. Each point must " "have one entry per world object (use None for a " f"component that should not be cropped), in order: {expected}.") + # Like astropy's world_to_pixel, match objects to components by class when unambiguous (#608). + vals = [v for v in point if v is not None] + slots = [[j for j, cls in enumerate(classes) if isinstance(v, cls)] for v in vals] + matched = {s[0]: v for v, s in zip(vals, slots) if len(s) == 1} + if len(matched) == len(vals): + points[i] = point = [matched.get(j) for j in range(len(comp))] for j, value in enumerate(point): if not (value is None or isinstance(value, classes[j])): raise TypeError(f"{type(value)} of component {j} in point {i} is " diff --git a/ndcube/tests/test_ndcube_slice_and_crop.py b/ndcube/tests/test_ndcube_slice_and_crop.py index 02aee0566..464b25601 100644 --- a/ndcube/tests/test_ndcube_slice_and_crop.py +++ b/ndcube/tests/test_ndcube_slice_and_crop.py @@ -472,6 +472,9 @@ def test_crop_by_extra_coords_all_axes_with_coord(ndcube_3d_ln_lt_l_ec_all_axes) output = cube.crop(lower_corner, upper_corner, wcs=cube.extra_coords) expected = cube[0, 0:2, 1:4] helpers.assert_cubes_equal(output, expected) + # 1 m matches both Quantity coords, so the point is matched by position. + output = cube.crop((None, None, interval2[0]), (None, None, interval2[1]), wcs=cube.extra_coords) + helpers.assert_cubes_equal(output, cube[:, :, 1:4]) def test_crop_by_extra_coords_values_all_axes_with_coord(ndcube_3d_ln_lt_l_ec_all_axes): @@ -619,6 +622,36 @@ def test_crop_all_points_beyond_cube_extent_error(points): cube.crop(*points, keepdims=True) +def test_crop_non_contiguous_world_objects(ndcube_4d_ln_l_t_lt): + # World axes are HPLT, TIME, WAVE, HPLN, so the SkyCoord components are not + # adjacent. The expected object order is first appearance: SkyCoord, Time, Quantity. + cube = ndcube_4d_ln_l_t_lt + lower = cube.wcs.array_index_to_world(1, 0, 0, 4)[0] + upper = cube.wcs.array_index_to_world(2, 0, 0, 5)[0] + output = cube.crop([lower, None, None], [upper, None, None]) + helpers.assert_cubes_equal(output, cube[1:3, :, :, 4:6]) + # Unambiguous objects are matched to components by class, so any order works (#608). + sky, _, wave = cube.wcs.pixel_to_world(1, 2, 3, 4) + helpers.assert_cubes_equal(cube.crop([wave, sky, None], keepdims=True), cube[4:5, 3:4, :, 1:2]) + # 1 matches no class, so the point is matched by position and rejected. + with pytest.raises(TypeError): + cube.crop([wave, sky, 1]) + + +def test_crop_by_values_many_axes_keeps_axis_order(): + # Pixel axes 3 and 8 used to be visited in set order ([8, 3]) and swapped. + wcs = WCS(naxis=9) + wcs.wcs.ctype = ["LINEAR"] * 9 + wcs.wcs.cunit = ["m"] * 9 + cube = NDCube(np.zeros((5,) * 9, dtype=bool), wcs=wcs) + point = [None] * 9 + point[3] = 1 * u.m + point[8] = 4 * u.m + output = cube.crop_by_values(point, keepdims=True) + world = output.wcs.low_level_wcs.pixel_to_world_values(*[0] * 9) + assert (world[3], world[8]) == (1, 4) + + def test_crop_by_values_quantity_table_coordinate(): # Regression: QuantityTableCoordinate-based WCS raised # "High Level objects are not supported with the native API" because diff --git a/ndcube/utils/cube.py b/ndcube/utils/cube.py index d1c188a13..abe420e7b 100644 --- a/ndcube/utils/cube.py +++ b/ndcube/utils/cube.py @@ -164,7 +164,7 @@ def get_crop_item_from_points(points, wcs, crop_by_values, keepdims, original_sh pixel_axes_with_input.append(point_inputs_pixel_axes[i]) pixel_axes_with_input = set(chain.from_iterable(pixel_axes_with_input)) pixel_axes_without_input = set(range(low_level_wcs.pixel_n_dim)) - pixel_axes_with_input - pixel_axes_with_input = np.array(list(pixel_axes_with_input)) + pixel_axes_with_input = np.array(sorted(pixel_axes_with_input)) pixel_axes_without_input = np.array(list(pixel_axes_without_input)) # Slice out the axes that do not correspond to a coord # from the WCS and the input point. From ac75a60494c688f9d6c1ef1c5e57493325482dfb Mon Sep 17 00:00:00 2001 From: Nabil Freij Date: Sun, 27 Sep 2026 15:36:45 -0700 Subject: [PATCH 3/5] Rename changelog fragments to the PR number --- changelog/{crop-object-order.bugfix.1.rst => 977.bugfix.1.rst} | 0 changelog/{crop-object-order.bugfix.rst => 977.bugfix.rst} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename changelog/{crop-object-order.bugfix.1.rst => 977.bugfix.1.rst} (100%) rename changelog/{crop-object-order.bugfix.rst => 977.bugfix.rst} (100%) diff --git a/changelog/crop-object-order.bugfix.1.rst b/changelog/977.bugfix.1.rst similarity index 100% rename from changelog/crop-object-order.bugfix.1.rst rename to changelog/977.bugfix.1.rst diff --git a/changelog/crop-object-order.bugfix.rst b/changelog/977.bugfix.rst similarity index 100% rename from changelog/crop-object-order.bugfix.rst rename to changelog/977.bugfix.rst From c851920afca6e416410f61c82b7b383cd95b2d7d Mon Sep 17 00:00:00 2001 From: Nabil Freij Date: Thu, 1 Oct 2026 12:02:00 -0700 Subject: [PATCH 4/5] Update 977.bugfix.1.rst --- changelog/977.bugfix.1.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/977.bugfix.1.rst b/changelog/977.bugfix.1.rst index 1bbb0e022..4a652b048 100644 --- a/changelog/977.bugfix.1.rst +++ b/changelog/977.bugfix.1.rst @@ -1 +1 @@ -Fixed `~ndcube.NDCube.crop` and `~ndcube.NDCube.crop_by_values` swapping the bounds of some pixel axes on cubes with nine or more dimensions. + ~ndcube.NDCube.crop and ~ndcube.NDCube.crop_by_values no longer swap the bounds of some axes on cubes with nine or more dimensions. From d3518cec2c4d6a48f473af04ac3ccb663f4340b8 Mon Sep 17 00:00:00 2001 From: Nabil Freij Date: Thu, 1 Oct 2026 12:02:52 -0700 Subject: [PATCH 5/5] Update 977.bugfix.rst --- changelog/977.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/977.bugfix.rst b/changelog/977.bugfix.rst index 5d92e9fbf..eec586f0f 100644 --- a/changelog/977.bugfix.rst +++ b/changelog/977.bugfix.rst @@ -1 +1 @@ -Fixed `~ndcube.NDCube.crop` rejecting points given in the order returned by ``pixel_to_world``, including partial points with ``None``, when a WCS's world objects are interleaved (e.g. ``HPLN, WAVE, HPLT``). Objects that each match exactly one world object class can now also be given in any order, as with `~astropy.wcs.wcsapi.BaseHighLevelWCS.world_to_pixel`, and the errors for a point of the wrong length or type list the expected objects in order. +~ndcube.NDCube.crop now accepts points in the order ``pixel_to_world`` returns, including partial points with `None`, when a WCS's world objects are interleaved (e.g. HPLN, WAVE, HPLT). If each object matches only one world object, the objects can be given in any order, as with ~astropy.wcs.wcsapi.BaseHighLevelWCS.world_to_pixel. Errors for a point of the wrong length or type now list the expected objects.