Fix world coordinates for 1D WCS

[astrofrog]

Found this while working on the tick labels in profile viewer.

[dhomeier]

@astrofrog some cases with result shapes like (1, N) needed an extra check to pass.
broadcast_to should have outlived its existence for some time now, although since it's not strictly private, there's perhaps some small risk to break something downstream (I did test glue-jupyter and glue-astronomy against this branch).

[astrofrog]

@dhomeier - I had to revert a1d114c in my latest commit - what use case required your fix?

[dhomeier]

This for example – result at that point was created from e.g. a length-3 array, but constructed as shape (1, 3), so the slicing result[0] was still required.
https://github.com/glue-viz/glue/actions/runs/3687605543/jobs/6241424695#step:9:584 or
https://github.com/glue-viz/glue/actions/runs/3903134088/jobs/6667109278#step:9:389

Maybe have to check additionally for something like result.shape[0] == 1 – what case was failing for you with the old commit?

[astrofrog]

Still a WIP, but getting closer

[astrofrog]

@dhomeier the remaining issue was that IdentityCoordinates, for single coordinates, should return a scalar or array, not a tuple with one element (this is prescribed by the APE 14 API)

[dhomeier]

Yes, that seemed weird, but it wasn't clear to me that came from inside glue.

[dhomeier]

But are you sure it is always a tuple? The self.pixel_n_dim == 1 case still does not seem to catch it.

[dhomeier]

a1d114c was triggered on np.ndim(result) > 1, that would mean that value is different from self.pixel_n_dim?

[astrofrog]

But are you sure it is always a tuple?

In principle APE 14 mandates that a tuple is returned if there are >coordinates. But there may still be cases where this doesn't happen?

The self.pixel_n_dim == 1 case still does not seem to catch it.

Is there a test failure still where this is not caught? Or can you come up with a failing example?

[dhomeier]

Is there a test failure still where this is not caught? Or can you come up with a failing example?

Strangely it seems to pass in most of the py310 envs, but none of the others. But on my Mac it's passing Pythons 3.9-3.11... Very weird.
I thought about directly testing for a tuple, but that is failing elsewhere.

[astrofrog]

I think this should be fixed - it was an issue with LegacyCoordinates. Not quite sure why it would fail only on some Python versions though... Let's see if the CI is happy.

[pllim]

I think this broke Jdaviz, see log at https://github.com/spacetelescope/jdaviz/actions/runs/3954350017/jobs/6771598108

ValueError: input operand has more dimensions than allowed by the axis remapping

[astrofrog]

Investigating!

[dhomeier]

Seems they are all failing on glue_astronomy.spectral_coordinates.SpectralCoordinates instances of shape (1, N) which are using their own pixel_to_world_values method (even if they were subclassed on LegacyCoordinates).

[dhomeier]

In principle APE 14 mandates that a tuple is returned if there are >coordinates. But there may still be cases where this doesn't happen?

Does this mean it must not return a tuple for the single coordinate case? Specifically should SpectralCoordinates with n_dim=1 always return np.interp(...)[0] or tuple(np.interp(...)[0])?
Either change does fix the jdaviz tests.

[astrofrog]

I have fixed this in a glue-astronomy PR, could you take a look at it? (Can't link to it on phone)

[pllim]

glue-viz/glue-astronomy#82

[pllim]

glue-core needs a release. We see failure in Jdaviz now because glue-astronomy is released but not this PR.

[dhomeier]

These are the updates currently in

Bug Fixes

• More fixes for compatibility with Qt6 by @astrofrog in More fixes for compatibility with Qt6 #2349
• Fix world coordinates for 1D WCS by @astrofrog in Fix world coordinates for 1D WCS #2345

Other Changes

• API update to numpy 1.24 by @dhomeier in API update to numpy 1.24 #2346

Still open

• Relax longitude range in fullsphere projections #2348 that might also go in.

But in addition there is a new regression in

• GlueSerializer fails on re-wrapped __array_function__ dispatcher in numpy-dev #2353. As that only concerns numpy-dev, it might not be as urgent yet.