AllenCellModeling / napari-aicsimageio

Multiple file format reading directly into napari using pure Python
GNU General Public License v3.0
33 stars 9 forks source link

consider loading first scene by default in scene widget #34

Open tlambert03 opened 2 years ago

tlambert03 commented 2 years ago

I love the new scene widget... but the other day I found myself looking specifically for a file that didn't have scenes, so that I could at least have an image loaded. What you you think about loading the first scene by default, in addition to showing the widget to select additional scenes? This keeps the image-loading process to "one step" for the relatively common use case of starting with scene0.

cc @psobolewskiPhD @JacksonMaxfield

psobolewskiPhD commented 2 years ago

This should be still the case. The logic I added checked if (n_scenes >1). On my machine testing with some LIF, CZI and TIFF I only get the selection widget (and the extra option widget) when s>1.

tlambert03 commented 2 years ago

oh! it's possible i was using a stale branch. lemme check again

psobolewskiPhD commented 2 years ago

Here's the relevant code: https://github.com/AllenCellModeling/napari-aicsimageio/blob/76059746a493154088d1d695ff87de09e1bbb4e6/napari_aicsimageio/core.py#L200-L215

tlambert03 commented 2 years ago

right, but after return [(None,)] ... where is the logic in _get_scenes that adds scene 0 to the viewer? I don't see it... and here's what I'm seeing locally (need to click on a scene to load it):

https://user-images.githubusercontent.com/1609449/135763144-00f1bcb7-0872-4299-bfd4-b04202549d36.mov

sample file: https://www.dropbox.com/s/w8bgza0g96swwza/dims_p4z5t3c2y32x32-tile2x3.nd2?dl=0

tlambert03 commented 2 years ago

oh, sorry, I reread your comment. I think you might have misunderstood my goal here. I'm proposing to load scene0 by default when n_scenes >1. not just to load the widget.

psobolewskiPhD commented 2 years ago

I'm ok with loading scene[0] automatically for multi-scene files, though for my use case I'd prefer this not to be the case (first few scenes are typically various staining controls I rarely want to analyze in napari). This could be an option perhaps? (Not sure the current checkbox approach is stored between sessions...) For sure for single-scene files the single scene should be automatically loaded—can you confirm this works with your nd2?

tlambert03 commented 2 years ago

For sure for single-scene files the single scene should be automatically loaded

yeah, that's definitely the case, that's why I was saying in the first post "I found myself looking specifically for a file that didn't have scenes, so that I could at least have an image loaded".

it's ok if you'd rather not have it load. just a thought. perhaps an option someday... but low priority

psobolewskiPhD commented 2 years ago

Oh derp, I understood that that combined with the next sentence that the scene it didn't load. Sorry! I would rather it not load, but I can understand it as a reasonable default đŸ˜„

tlambert03 commented 2 years ago

current behavior is like bioformats in Fiji, where one has to do an additional step if scenes are detected... so there's certainly precedent for your preference! let's leave it as default, and if I care enough someday I'll make it configurable :)

psobolewskiPhD commented 2 years ago

BTW: Is there some thought to napari providing/storing plugin preferences, so such behavior could be configured and stored between sessions?

tlambert03 commented 2 years ago

yep, will come with the new manifest as a "configuration" contribution here. Will roughly follow the vscode spec described here.

In the meantime, plugins would have to do their own persistence. (magicgui helps with that sort of thing using the persist parameter to @magicgui https://napari.org/magicgui/api/magicgui.html)

evamaxfield commented 2 years ago

I tend to agree that we should just default to scene 0 on load. It would be great to have all the parameters persist and I would argue that we split this issue up into two issues / PRs. (This also helps down the line because I want to add parameters for "load lazy vs load in-memory" and having that parameter persist would be amazing.)

  1. adds persistance to parameters
  2. adds toggle for scene 0 loading. ("Load Scene 0 on File Selection" or similar naming)