Skip to content

Conversation

nabobalis
Copy link
Member

No description provided.

@DanRyanIrish
Copy link
Member

DanRyanIrish commented Oct 2, 2025

Hi @nabobalis. It looks to me that the time world axis is also coupled to the celestial world axes via a common pixel axis. So in order to unambiguously determine a point in the cube associated with a time, you also have to provide a SkyCoord, which you haven't done. That's why the error message is saying that it expects 2 inputs ((SkyCoord, Time)) when you have only given it one (Time).

I'm not sure that there is a general solution that gets around this problem with this WCS. Alternatively, you could use the cube.extra_coords.cube_wcs instead of combined_wcs. Since this adds dummy world axes in units of pixels for pixel/array axes not associated with any extra coords, its output of

tuple(val[0] for val in cube.extra_coords.cube_wcs.world_axis_object_classes.values())

is

(Time, Quantity, Quantity)

In that case, I think your corners would become:

    lower_corner = (Time("2000-01-01T15:00:00", scale="utc", format="fits"), None, None)
    upper_corner = (Time("2000-01-01T20:00:00", scale="utc", format="fits"), None, None)

@nabobalis
Copy link
Member Author

nabobalis commented Oct 2, 2025

With that change, I get:
test_crop_by_extra_coords_using_combined_wcs - ValueError: 3 components in point 0 do not match WCS with 4 components.

If I add another None,

TypeError: <class 'astropy.time.core.Time'> of component 0 in point 0 is incompatible with WCS component spectral_0 <class 'astropy.units.quantity.Quantity'>.

In that block, the order seems to be ['spectral_0', 'celestial_0', 'temporal_1', 'PIXEL_1']

@nabobalis nabobalis marked this pull request as ready for review October 9, 2025 16:33
@nabobalis nabobalis added the No Changelog Entry Needed Skip all changelog checks. label Oct 9, 2025
@nabobalis nabobalis added this to the 2.4.0 milestone Oct 9, 2025
@nabobalis nabobalis changed the title Added failing unit tests Added specific cropping unit tests Oct 9, 2025
Copy link
Member

@DanRyanIrish DanRyanIrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor name changes to the tests to better describe what that are actually testing.

@DanRyanIrish DanRyanIrish merged commit 8d2a20c into main Oct 13, 2025
20 checks passed
@DanRyanIrish DanRyanIrish deleted the wcs_crop_combined branch October 13, 2025 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No Changelog Entry Needed Skip all changelog checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants