Skip to content

Support N-D table coords, subclass attr propagation, clearer crop error#938

Closed
nabobalis wants to merge 1 commit into
sunpy:mainfrom
nabobalis:fix
Closed

Support N-D table coords, subclass attr propagation, clearer crop error#938
nabobalis wants to merge 1 commit into
sunpy:mainfrom
nabobalis:fix

Conversation

@nabobalis

Copy link
Copy Markdown
Member

PR Description

Allow a single N-D Time or Quantity lookup table in extra_coords, representing one coordinate varying over several pixel dimensions. Add NDCube._extra_attrs_to_copy so subclasses can declare attributes propagated through arithmetic and to_nddata. Make the crop component-count error name the expected world objects.

Description

This PR upstreams three improvements motivated by irispy-lmsal's gWCS raster
work, where IRIS spectrograph observations are represented as 4-D
(scan, step, slit, wavelength) cubes. Each change removes a workaround that
instrument packages currently have to carry themselves.

1. N-D Time/Quantity lookup tables in extra_coords

TimeTableCoordinate and QuantityTableCoordinate now accept a single N-D
table, representing one physical coordinate that varies over N pixel
dimensions. For example, a 2-D Time table indexed by (raster scan, raster
step) can now be attached with:

cube.extra_coords.add("time", (0, 1), times_2d, physical_types="time")

Previously QuantityTableCoordinate raised "Currently all tables must be
1-D" and TimeTableCoordinate was hardwired to one dimension, so per-exposure
times on a multi-scan cube could not be expressed as an extra coord at all.

Implementation notes:

  • N-D tables expose their model inputs in pixel order (reversed array
    order). NDCube._generate_world_coords transposes world values assuming
    the APE-14 pixel-axis convention, so an array-ordered model produces
    transposed axis_world_coords output. ExtraCoords.mapping reverses the
    converted axes for such coordinates to stay consistent (see the new
    BaseTableCoordinate._model_inputs_are_pixel_ordered property).
  • ExtraCoords._getitem_lookup_tables now drops integer-sliced axes from a
    table's axes tuple when the sliced table actually loses pixel dimensions.
    Previously the axes tuple kept stale (even negative) entries; this was
    latent for existing coordinate types but breaks N-D tables. Range slices
    keep the table N-D, integer slices reduce it, and slicing away all table
    dimensions drops the coordinate into dropped_tables as before.
  • interpolate supports N-D tables via scipy.interpolate.interpn
    (TimeTableCoordinate.interpolate now takes *new_array_grids, matching
    the other table coordinate types; the old single-grid call still works).
  • Fixed a latent Length1Tabular dispatch bug: a table of shape (1, n) was
    routed to the length-1 1-D model because the check used len(table) == 1
    instead of table.shape == (1,).

2. NDCube._extra_attrs_to_copy extension point

Subclasses that carry domain-specific instance attributes (observer
coordinates, per-step WCS tables, instrument flags, ...) currently have to
override _new_instance and to_nddata to keep those attributes from
silently disappearing on every derived cube. Forgetting one call site means
arithmetic or a utility function returns a cube with the metadata gone and no
error.

Subclasses can now declare:

class MyCube(NDCube):
    _extra_attrs_to_copy = ("observer", "calibration_level")

and the attributes are propagated (by reference) through _new_instance
(arithmetic, __neg__, fill_masked, ...) and through to_nddata when the
target nddata_type is a subclass carrying the same attributes. Explicit
kwargs still override the automatic copy, and plain NDData targets are
unaffected. Slicing is deliberately not covered: shape-dependent attributes
cannot be sliced generically, so subclasses keep handling that themselves
(documented on the attribute).

3. Crop component-count error names the expected world objects

Old code written against a previous WCS fails with
"2 components in point 0 do not match WCS with 4 components.", which gives
the user no clue what the four components are. The error now reads:

2 components in point 0 do not match WCS with 4 components. Each point must
have one entry per world object (use None for a component that should not be
cropped), in order: em (SpectralCoord), celestial (SkyCoord), time (Time),
custom_step (Quantity).

The existing message prefix is preserved, so downstream tests matching on it
keep passing.

Testing

  • 13 new tests: N-D table construction, WCS round-trips, slicing (range,
    integer, full drop), interpolation, multi-N-D rejection, cube-level
    axis_world_coords through extra_coords including after slicing,
    attribute propagation through arithmetic and to_nddata, and the new crop
    error wording.
  • Full ndcube suite passes locally (534 passed; one pre-existing
    environment-dependent failure with a newer gwcs,
    test_crop_by_extra_coords_values_all_axes_with_coord, fails identically
    on main).
  • Validated against irispy-lmsal's gWCS raster branch: its full suite passes
    against this patch, and the 2-D (scan, step) exposure-time table that
    motivated change 1 attaches and slices correctly on a real 13-scan
    observation.

Notes for reviewers

  • The pixel-ordering convention for N-D table models (point 1 above) is the
    part most worth scrutinizing: multi-1-D-table coordinates (e.g. meshed
    SkyCoord tables) keep their existing array-ordered convention, and only
    single N-D tables use the reversed order. An alternative would be to fix
    _generate_world_coords to consult the mapping instead, but that touches
    every WCS path rather than only the new feature.

AI Assistance Disclosure

AI tools were used for:

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding
  • No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

Allow a single N-D Time or Quantity lookup table in extra_coords,
representing one coordinate varying over several pixel dimensions.
Add NDCube._extra_attrs_to_copy so subclasses can declare attributes
propagated through arithmetic and to_nddata. Make the crop
component-count error name the expected world objects.
@nabobalis

Copy link
Copy Markdown
Member Author

Honestly, this was created to make my life easier in creating a 5D gWCS. I don't even know if these changes make logical sense in the first place.

@nabobalis nabobalis closed this Jun 11, 2026
@nabobalis nabobalis deleted the fix branch June 11, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant