Skip to content

Conversation

@Carifio24
Copy link
Member

@Carifio24 Carifio24 commented Oct 17, 2025

While looking into some 3D viewer stuff related to e.g. glue-viz/glue#2559, I noticed that the new approach to handling pixel component IDs introduced in #399 has a few cases that it misses.

In particular, equivalent_pixel_cids I think is too restrictive for the purposes of the data proxy FRB calculation. In particular, this runs into problems if we link the two dataset on the associated world coordinates, rather than the pixel coordinates directly - i.e. if I have two datasets A and B, and link my coordinates like

Pixel A <-- world2pix/pix2world --> World A <-- My link --> World B <-- world2pix/pix2world --> Pixel B

then the current approach won't find the datasets as compatible, even though this should be enough to display e.g. layer B if layer A is my reference data. The solution used here is to instead find the pixel component ID matrix and look at the relevant indices of true values. If nothing maps to a given index we just put None into the order.

@astrofrog does this seem reasonable?

@Carifio24 Carifio24 requested a review from astrofrog October 17, 2025 17:53
@Carifio24
Copy link
Member Author

@astrofrog I've updated the FRB calculation in the data proxy to omit finding the order per your suggestion, I agree that that's a cleaner (and less error-prone) way to do things.

I left this in the data proxy's shape calculation, as I'm not sure of another way to know which particular axis of the underlying array to access to get the shape. But if there is a simpler way then I am glad to update that as well

@astrofrog
Copy link
Member

@Carifio24 - it looks like the test failures are actually related:

https://github.com/glue-viz/glue-vispy-viewers/actions/runs/18658021138/job/54702008413?pr=401

Most of the Qt jobs are failing. The ones that pass are all Windows weirdly. Would you be able to take a look, and ping me if you are not able to resolve any of the failures?

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.

2 participants