feature/scene-widget-additions

[evamaxfield]

My continuation from #25

My additions include a checkbox that lets the user determine if they want to clear the current layers before scene selection change.
Additionally, I unpack the channels as layers just like we do for the single scene files.

You can see the new functionality in this gif: https://github.com/AllenCellModeling/napari-aicsimageio/blob/psobolewskiPhD-feature/scene-widget/images/scene-selection.gif

@psobolewskiPhD and @tlambert03 would love your feedback on my minor changes if you have any comments.

[codecov-commenter]

Codecov Report

Merging #27 (48af7ef) into main (8f298c6) will increase coverage by 8.50%.
The diff coverage is 93.69%.

@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
+ Coverage   74.54%   83.05%   +8.50%     
==========================================
  Files           7        5       -2     
  Lines         110      177      +67     
==========================================
+ Hits           82      147      +65     
- Misses         28       30       +2     

Impacted Files	Coverage Δ
napari_aicsimageio/in_memory.py	0.00% <0.00%> (ø)
napari_aicsimageio/out_of_memory.py	0.00% <0.00%> (ø)
napari_aicsimageio/core.py	85.45% <96.38%> (+10.04%)	⬆️
napari_aicsimageio/tests/test_core.py	100.00% <100.00%> (ø)
napari_aicsimageio/__init__.py
napari_aicsimageio/tests/__init__.py

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f298c6...48af7ef. Read the comment docs.

[psobolewskiPhD]

Everything works, but I'm not 100% sold on the new features:

My additions include a checkbox that lets the user determine if they want to clear the current layers before scene selection change.

For me the default (box checked behavior) is odd. First, I was expecting that this setting would clear the selected scene from the scene list widget, so essentially pruning scenes that were already open. I was not expecting it to clear the image I just selected from the image layer list.
For me, whole point of the scene selection widget it to let me open as many scenes as I want, by just clicking through a list. I can then pick and choose if I want to delete an image layer. But this is largely my use case: screening a file with 100 scenes to select sequences of scenes for analysis, for example. I heavily make use layer visibility—killer napari feature for me—to make comparisons between images, so I want many scenes open at once.

Additionally, I unpack the channels as layers just like we do for the single scene files.

Again, for me this is a regression. When browsing files with multiple scenes, I want to have things grouped by scene, so that's the logical unit for me, not the channel. Worse yet, I anticipate that multiple scene files will frequently—typically?—have the same channels. In my case, they always have the same channels, so I get image layers:

Green--FLUO--DAPI [3]
Green--FLUO--DAPI [2]
Green--FLUO--DAPI [1]
Green--FLUO--DAPI

Then I have no way of knowing what I'm looking at—what scene the image came from—because the scene name is not passed to the layer name.
Beyond the name issues, I'd rather scenes were imported as channel stacks, because when browsing multiple scene files I want multiple scenes open at once. It gets chaotic with mutliple channels and multiple scenes. Once I have scenes selected, I can click on the scene in the image layer list and split it, if I want.

Anyhow, both of these features are likely to be very conditional on the specific use case. Some may prefer your default behavior, others—me!—may prefer mine. So if there is already to be a widget with some settings, then perhaps the channel behavior could also be controlled with a setting:
[ ] Unpack channels on import.

[evamaxfield]

First: very fair points.

Second: if I added a checkbox for "Unpack channels" and left it unchecked by default would that be okay?

If so I will do that and then set the defaults to "unchecked" and the default functionality will be your preferred method.

[psobolewskiPhD]

Sounds good to me—any idea if these kind of checkbox settings can be preserved as preferences? Maybe @tlambert03 can chime in. Both boxes unchecked would be my preferred default, so I'm good to go, but someone else may prefer a default of both boxes check, so preserving the state, if possible, would be nice to have—perhaps down the road.

[evamaxfield]

Sorry for getting carried away at @psobolewskiPhD you taught me alot in opening this PR as to how to add more functionality to this plugin and now I can see a world where all image loading, regardless of single or many scene goes through this type of widget because it would allow the user to set all sorts of settings.

The major ones would be: Chunk dimensions "TYX" instead of our always default "ZYX" and to remove the two plugin variants in favor of simply having a check box that says "in memory" or "delayed"

Anyway, the new default functionality is in. Will record a GIF in a second!

[evamaxfield]

With tests passing. If you could give this branch a test on some of your files I would greatly appreciate it. All of the test files aren't the "coolest" images.

[evamaxfield]

New gif of functionality here: https://github.com/AllenCellModeling/napari-aicsimageio/blob/psobolewskiPhD-feature/scene-widget/images/scene-selection.gif

If all tests pass and I get the okay from you @psobolewskiPhD then will merge and release a new version.

[psobolewskiPhD]

Gah, lost my comment.

I've played around with it with my files. Works well! The checkboxes are both useful and play well with each other (opening packed or unpacked).
I have a small preference for preserving the scene index in the image layer name—it's just so handy to refer by the number later, as my scenes have often clunky names (B3 60 10X B/3/R45 etc.). See:

But it's not a hold up IMO.

Another idea I'll probably look into—at some point—is using:
list_widget.setSelectionMode(2) to have multiple scenes highlighted. It works, but of course will require different event triggers, because the current change triggers the same open-scene, regardless of select/deselect status. So a new connect would be needed for select event to open the scene and a deselect one to clear it. But it could be a nice UI polish.

The major ones would be: Chunk dimensions "TYX" instead of our always default "ZYX" and to remove the two plugin variants in favor of simply having a check box that says "in memory" or "delayed"

I love this idea. I'm not a fan of modal dialogs where you pre-set such things and then they are fixed. It's easy to imaging wanting to load different scenes with different settings. I really like the idea of the widget giving access to those settings live.
I think this provides nice use cases for @jwindhager napari issues: napari/napari#2202 and napari/napari#2229

[evamaxfield]

I have a small preference for preserving the scene index in the image layer name—it's just so handy to refer by the number later, as my scenes have often clunky names (B3 60 10X B/3/R45 etc.)

I will quickly add the scene name and index to the layer name. These layer names getting long hahaha.

I love this idea. I'm not a fan of modal dialogs where you pre-set such things and then they are fixed. It's easy to imaging wanting to load different scenes with different settings. I really like the idea of the widget giving access to those settings live.
I think this provides nice use cases for @jwindhager napari issues: napari/napari#2202 and napari/napari#2229

Absolutely agree. If reception to this plugin release goes well I can assume we can make a patch to make this the default load style which would allow us all that customization.

[evamaxfield]

Scene index now always shows:

edit: realized i can show both scene index and name so doing that.

[joshmoore]

New GIF home: https://github.com/AllenCellModeling/napari-aicsimageio/blob/main/images/scene-selection.gif