[builtins] handle reading mixed layer directories (e.g. with .csv)

[psobolewskiPhD]

References and relevant issues

Closes #5460

Description

This PR handles reading the output of write_layer_data_with_plugins ('Save to folder' in the GUI).

If a directory is passed to the reader, then a check is made whether it contains .csv files. If so, it reads each file in the directory using either the csv reader or the magic imreader.
The layer names are set to the file names (but source will still show the folder name).
If a file type cannot be handled, it is skipped with a notification.
Otherwise, the behavior is left unchanged.
The goal here is not universal handling of complex folder structures, just handling the common case of folders saved by napari containing layers.
I added a test to reflect this.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.44%. Comparing base (e29593c) to head (57ca513).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6890      +/-   ##
==========================================
- Coverage   92.49%   92.44%   -0.05%     
==========================================
  Files         612      612              
  Lines       55169    55207      +38     
==========================================
+ Hits        51027    51035       +8     
- Misses       4142     4172      +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DragaDoncila]

@psobolewskiPhD so sorry for the delay in getting to this... I've been playing with it today and I have some comments.

With napari==0.4.19.post1, when I drag a folder with 2 tiffs and a csv in, it stacks the tiffs with the csv, and when I try to scroll to the third slice, it errors. Is this the behavior you see? I couldn't quite tell from the issue whether you see the error with nothing open or whether it stacks.

With this PR, the tiffs don't stack, and I think that's a significant change. What do you think about stacking tiffs but reading csvs individually, since as you mention in the issue, csvs don't stack.

In general I'm worried about always trying to read in the csvs, but I totally see what you're saying about fixing reading for things we save out. @jni thoughts? I think maybe this is better than nothing, with yet another comment on us needing to do a bigger I/O overhaul...

[psobolewskiPhD]

@DragaDoncila I should update the issue, but the bug you observe is something that came up recently when working with an end user that had 2 tifs (image, labels), and a csv (points).
I tired to ensure that this PR IMO resolves that, by not stacking if there is a csv. The goal is to handle outputs of napari save to folder, which is super nice -- just need to get the svg issue resolved #6846 , so people know about it!
(I've just told people to uninstall the svg plugin, it's not really functional or maintained at this point anyways)
If someone has a folder of tiffs they shouldn't be affected.
(I mean the auto stack is hit or miss anyways. But it's beside the point, the default has been that way for a while.)
If they have a folder of tiffs with random other stuff in them, then the current behavior isn't good either.

[DragaDoncila]

The goal is to handle outputs of napari save to folder, which is super nice

Yeah I'm fully onboard with this goal, I'm just concerned that this changes behavior for all folders, not just those saved by napari. If people are relying on the stacking, it could be pretty grim to now get 600 layers trying to open because you have timelapse data with one tiff per frame (this is what I work with so it's close to my heart 😆 ). Maybe we do want to associate some metadata when we save... 😬 That's basically the only way we can guarantee a return trip

[psobolewskiPhD]

Is there any case where napari currently works "as expected" but this would break that?
Because a folder of tiffs or whatnot as a time series is not affected.

Edit: but yes, in general there is no way for us to figure out intent without some .file with metadata or a modal dialog saying "do you want this to be stacked or what?"

[DragaDoncila]

Because a folder of tiffs or whatnot as a time series is not affected.

Right, I guess it's unaffected so long as it doesn't happen to have a csv in it, I just think storing features etc. in these folders is kinda common? But maybe that's just me being messy 😆

[psobolewskiPhD]

Could adda heuristic that if there are more than n non-csv files that have the same extension, then stack em? But what would be n? 5? 10?

Edit: 30+ channels are totally possible now, so i dunno

[DragaDoncila]

@psobolewskiPhD yeahhh I thought along those lines too but it immediately spiked my blood pressure and I shied back away 😆 . Would be good to get more input from folks.

[psobolewskiPhD]

Other option: Open Folder as Stack menu item
Less magic, more explicit.
Edit2: and maybe rename the other Open Folder as Layers?
Edit3: for drag-n-drop there already is a keybind to do that, but i forget what it is or how well documented.

[DragaDoncila]

Other option: Open Folder as Stack menu item
Less magic, more explicit.
Edit2: and maybe rename the other Open Folder as Layers?

This would work, but we'd have to change the drag and drop behavior too, probs by also using Shift, same as we do for Files...

[psobolewskiPhD]

That sounds reasonable though, makes it consistent -- dragging a batch of files or a folder of the same files should work identically, with the same options.

[DragaDoncila]

Yeah I think that sounds sorta reasonable...

[psobolewskiPhD]

OK, ill put this in draft and poke around.
So the idea is:

1. Have Open Folder > (sub menu)
   - as Layers
   - as Stack
2. Have drag-n-drop of folder use keybind to modify the behavior as above.

For the stack case, do we just ignore non-image files? or error if there are more then 2 extension?

[psobolewskiPhD]

Of course zarr and other things will be annoying to handle or explain. 😡
Frankly, I have no idea whether a zarr should be as a file or as a folder.

[DragaDoncila]

Frankly, I have no idea whether a zarr should be as a file or as a folder.

if we use the system file browsing dialogs I think there's basically no way to tell them to open a zarr folder from the Open File menu... But yeah ideally you could just select anything and we would know under the hood whether it was a folder or not, but the user wouldn't have to care

[psobolewskiPhD]

@DragaDoncila I spent some time on this but as far as I can tell npe2 readers don't get stack passed to them anywhere?
magic_imreader just has stack=True set by default
So with a folder, which is a single path string, there is no way to pass a stack variable that could be used to handle whether the contents should be stacked or not.

[DragaDoncila]

@DragaDoncila I spent some time on this but as far as I can tell npe2 readers don't get stack passed to them anywhere?

Gahhhh yes I think you're right. There was a push at once stage to get that through to the readers but hasn't been exposed. Currently the "idea" is that if you get a list of images you're supposed to stack them, and if you get just one folder you're not. But I don't know to what extent, if at all, that convention is followed. We should update the manifest, it's just a matter of what that's going to mean for older readers that can't take the stack argument at all

[psobolewskiPhD]

Any thoughts where that leaves this PR?

[jni]

"do you want this to be stacked or what?"

🤣 I will insta-merge a PR adding this modal 😂

For the stack case, do we just ignore non-image files?

I think yes.

Frankly, I have no idea whether a zarr should be as a file or as a folder.

zarr v3 should help here because the files will be zarr.json instead of .zarray/.zgroup/.zattrs. So we could instruct people to open zarr.jsons and not the containing folder.

In the meantime, it has to be a folder, indeed.

Any thoughts where that leaves this PR?

Wow it's super tricky. 😂 One idea: provide two different plugins, builtins-layers and builtins-stack?

Another idea (more out there): when we save all layers to folder, we save a napari-layers.yaml file within the folder that contains all the layer info (this file makes up this layer etc) and tells the builtins plugin to do the right thing? (And/or maybe we instruct users to open that file directly?)