Repository navigation
Crop mishandles negative array indices #860
Description
Describe the bug
NDCube.crop uses wcs.world_to_array_index to convert world coordinates to array indices. This function uses negative array indices to denote points to the left/below the extent of the data array. NDCube.crop naively uses the returned array indices to construct the required slice item. This can result in a slice item of slice(-1, 1) which is intended to be equivalent to slice(0, 1). However, Python's slicing notation interprets this can slicing forwards from the last array index to the first. This results in slicing the cube to a shape of (0,)
Proposed solution:
ndcube.utils.cube.get_crop_item_from_pointsshould convert any negative array index to 0, so that it's output is alwaysslice(0, 1), and neverslice(-1, 1)NDCube.__getitem__should add to its slice input sanitization so that it errors for cases likeslice(-1, 1)where the result will be shape(0,)
Thanks to @hayesla for revealing this bug.
To Reproduce
import astropy.units as u
import numpy as np
import sunpy.map
from astropy.coordinates import SkyCoord
from astropy.wcs import WCS
from ndcube import NDCube
def make_simple_cube():
"""
A simple 9x9 map, with its center at (0, 0),
and scaled differently in each direction.
"""
data = np.arange(81).reshape((9, 9))
ref_coord = SkyCoord(0.0, 0.0, frame='helioprojective', obstime='2020-01-01 00:00:00', unit='deg',
observer=SkyCoord(0 * u.deg, 0 * u.deg, 1 * u.AU, frame='heliographic_stonyhurst'))
ref_pix = [4, 4] * u.pix
scale = [2, 1] * u.arcsec / u.pix
header = sunpy.map.make_fitswcs_header(data, ref_coord, reference_pixel=ref_pix, scale=scale)
return NDCube(data, WCS(header))
cube = make_simple_cube()
# Create world coords to crop to.
bottom_left = cube.wcs.array_index_to_world(-1, -1)
top_right = cube.wcs.array_index_to_world(0, 0)
# Crop cube to 0-size
cube0 = cube.crop((bottom_left,), (top_right,))Metadata
Metadata
Assignees
Labels
Type
Projects
- Status
Activity
DanRyanIrish commentedon Aug 13, 2025
The
croprelated part of this issue is addressed by PR #874.The
__getitem__part is currently being addressed by astropy PR #18519. If this is accepted, an additional change toNDCube.__getitem__will be needed to leverage the new astropy feature, andndcubemay need its minimum astropy version pinned to the the next release of astropy containing this fix.DanRyanIrish commentedon Aug 14, 2025
FYI @hayesla, I took on this issue because I realised it would be best to facilitate it through changes to astropy (#18519) and #874. However, it will still need a simple PR to ndcube when these changes are accepted. It would be great to have you lead that PR if you're still interested.
DanRyanIrish commentedon Aug 20, 2025
@Cadair pointed out that, in addition to the fix in astropy, we will need an equivalent fix in ndcube to get the fix available sooner and to continue supporting older versions of astropy for the expected life-cycle.
Fix slicing with negative and out-of-range indices
Raise IndexError for slices that would give a length-0 axis
Fix slicing with negative and out-of-range indices
Raise IndexError for length-0 and stepped slices
Fix slicing with negative and out-of-range indices
Raise IndexError for length-0 and stepped slices