Conversation
ExtraCoords._getitem_lookup_tables treated any axes that were not a tuple as a single axis, and kept every axis of a table after integer slicing. Tables added with a list of axes, and every multi-axis lookup table loaded from ASDF (which stores the axes as a list), therefore raised "list indices must be integers or slices" when the cube was sliced. Integer indexing also left stale axes on the table (cube[1] turned (0, 1) into (-1, 0)), so on a cube with more axes than the table the extra coord was read along the wrong array axis, giving wrong or NaN values. Normalise the axes to a tuple and drop integer-sliced axes.
_generate_world_coords transposed each block of correlated world coordinates with .T, which is only right when the WCS pixel inputs are in ascending cube pixel order. That holds for the cube's own WCS, but an ExtraCoords WCS takes its inputs in the order of its mapping, which for lookup tables follows the order the array axes were given in. So a 2-D SkyCoord table added on array axes (0, 1) came out of axis_world_coords transposed, as did WCS-backed extra coords whose mapping reorders correlated pixel axes. Order each block by descending cube pixel axis instead. For the cube's own WCS this is the same reversal as .T, so its output is unchanged.
This branch has not been deployed
This file contains hidden or 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
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.
PR Description
Slicing (
ExtraCoords._getitem_lookup_tables): table axes given as a list were treated as a single axis, so any slice raised TypeError. ASDF stores the axes as a list, so this broke every such table loaded from ASDF. Integer indexing also kept the dropped axis ((0, 1) became (-1, 0)), which gave the wrong length, or NaN, on a cube with more axes than the table.Transposed output (
_generate_world_coords): correlated world coords were transposed with a plain .T. That is only right when the WCS pixel inputs are in ascending cube-axis order. This holds for the cube's WCS, but not for an ExtraCoords WCS, whose inputs follow ExtraCoords.mapping. Soaxis_world_coords(wcs=cube.extra_coords)transposed a 2-D table on (0, 1), and also WCS-backed extra coords whose mapping reorders correlated axes. The transpose now follows the mapping. For the cube's own WCS it is still a full reversal, so normal WCS output is unchanged; the existing axis_world_coords tests check that.main:
this branch:
Related to #342
AI Assistance Disclosure
AI tools were used for:
K