Rehome subsets after removing data from viewer.

[rosteen]

This builds off of Duy's work to delete data from the data collection. Once I've figured out/debugged the relevant subset rehoming code I'll split just that out into it's own PR.

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
...nfigs/cubeviz/plugins/tests/test_data_selection.py	100.00% <100.00%> (ø)
jdaviz/configs/imviz/helper.py	97.38% <100.00%> (+0.02%)	⬆️
jdaviz/configs/imviz/tests/test_delete_data.py	100.00% <100.00%> (ø)
jdaviz/configs/specviz/tests/test_helper.py	100.00% <100.00%> (ø)
jdaviz/app.py	86.02% <94.80%> (+0.67%)	⬆️

... and 4 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

[rosteen]

Here's a video of this in action. You can see that I hit the bug reported in #2085 at the end of this, or at least a very similar one.

Screen.Recording.2023-09-18.at.3.11.45.PM.mov

[rosteen]

Marked this as ready for review, I think it's ready for people to try breaking it. I could still break out my code (the subset rehoming) into a separate PR from Duy's code to allow deleting data if there are other issues still to address with data deletion, but as far as I recall the subset problem was the main thing holding the deletion back, so we could potentially merge both together with this PR.

[camipacifici]

I tried, but I do not see the same behavior as in the video. The subsets are not showing anymore and the subset plugin still points to the original coordinates. I was using multiple viewers...maybe that caused some glitch. Here is a screenshot.

[rosteen]

I didn't consider that case, I'll have to refactor a bit. I was actually re-parenting the subsets in remove_data_from_viewer and looping through the layers in that viewer, but that won't catch data that's in another viewer. I'll see if it works to move the logic to happen when the data is actually deleted and loop through any existing viewers.

[rosteen]

@camipacifici I think I fixed that case. I realized that that re-parenting the subsets might only be possible if the user deletes the data from the data selection menu UI. If you just remove the data from the data_collection in the notebook, we'll get a DataCollectionDeleteMessage but at that point it may be too late to use the data to translate the subset bounds through WCS (because the data is already gone). I could be wrong on exactly why it was failing, but it was failing in a pretty messy way when I had this logic in the handler for DataCollectionDeleteMessage.

[rosteen]

Docs build failure is an unrelated timeout.

[pllim]

The print functions have to be removed before merging. If you want to keep them for debugging, consider the snackbar in debug mode or proper logger.

The new logic starting around L1884 in app.py, I am not sure about. The explicit checks for certain shapes would exclude all composite subsets, no? Even in Imviz, we still allow people to create weird composite subsets even though they cannot do much more with them besides grabbing them back as list of Region objects (minus the operators).

As for testing, I think all the viz needs to be tested. I can see this being a widely used feature, especially when the performance starts to slow with all them links.

[javerbukh]

This all was in the Imviz notebook: when removing layer A (reference data) from the viewer, I see the subset move to layer B. I still see it's parent as layer A. However, if I then delete layer A, the subset is alone in the viewer with the viewer appearing as a black screen. In this case, the parent is now layer B.
If I repeat the first step and then link by WCS, the subset moves to a different part of the image.

Is all of this intended functionality? I think the subset should not be on it's own at any point. Does linking by a different type actually move the subset to a correct location (as the user would understand it)?

[rosteen]

Dev test failure is unrelated, due to scipy using something deprecated from numpy.