search

glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
721 stars 152 forks source link

Avoid over-aggressive link deletion on data removal #2406

Closed rosteen closed 1 year ago

rosteen commented 1 year ago

This fixes the bug shown in https://github.com/spacetelescope/jdaviz/pull/2194#issuecomment-1553077073, although I suspect that a better fix may lie elsewhere, possibly in the subset code rather than the link code. Essentially, we were seeing that after removal of the original data from the data collection, subset layers that applied to other data were no longer appearing, because their attributes were still set to the components of the removed data, but the links between those components and the components of the remaining data had been removed. This adds a check to see if any subsets are still using the links before removing them.

As I said, I suspect that a better fix would be to update the attributes of the remaining subsets to the components of remaining data when the original data is removed, but this is the first thing I could get working to fix this. So, feel free to reject this if it doesn't seem like a good solution.

pllim commented 1 year ago

cc @dhomeier and @astrofrog

dhomeier commented 1 year ago

There is one test failure in py39-test-pyside515 that did not occur before: https://github.com/glue-viz/glue/actions/runs/5018272672/jobs/8997499750?pr=2406#step:10:418

It's an allowed failure, and all PySide tests are failing for various other reasons, but this one is related to the LinkEditor and might be worth investigating.

astrofrog commented 1 year ago

There is definitely an issue here that needs fixing! I'm not too keen on preserving the links and component IDs etc without the actual parent dataset they were originally associated with, so I'd like to suggest an alternative fix for discussion - PR forthcoming!

rosteen commented 1 year ago

Closed this since it ended up not being the desired solution.