Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support opening of masks #30

Merged
merged 6 commits into from
Jul 10, 2020
Merged

Support opening of masks #30

merged 6 commits into from
Jul 10, 2020

Conversation

will-moore
Copy link
Member

Currently supports opening of local zarr masks.

To test:

  • Export masks as described in Add mask-map logic omero-cli-zarr#11
  • Open image in napari e.g. $ omero napari view Image:5514375
  • In napari console, do viewer.open('path/to/5514375.zarr/masks/')

Screenshot 2020-07-03 at 16 51 59

@will-moore will-moore changed the title Support opening of local masks Support opening of masks Jul 6, 2020
@will-moore
Copy link
Member Author

With that last commit, and with fsspec patched as described with @joshmoore's workaround from fsspec/filesystem_spec#342 (comment)

napari 'https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001240.zarr/'

loads masks along with the Image:

Screenshot 2020-07-06 at 17 18 31

And this works locally (with images exported as above) loading masks with the Image:

$ napari 5514375.zarr

Screenshot 2020-07-06 at 17 21 24

@joshmoore
Copy link
Member

In testing against a local array, I was failing to load the masks, @will-moore. I tried unifying the logic and this now works. This, however, removes your directory listing code. Two thoughts:

  • shall I add it back optionally? i.e. if the server doesn't allow it it isn't loaded
  • or do we need to provide a file-browser for the the zarr itself, i.e. if enabled, you can see all the arrays that are in the zarr and open/overlay those that you want to.

cc: @tlambert03

masks: unify remote and local handling
ome_zarr.py Outdated
data = da.from_zarr(mask_path)
# mask data is 5D (t, c, z, y, x) but each layer in napari is 4D (no C)
# NB: Assume we want 'first Channel'
data = data[:,0,:,:,:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be handled on the napari side, instead of in this library?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah interesting. There does seem to be a limitation here in napari if you have multiple (image) channels along with a mask for each one. In the near(ish) future, we'll have layer "groups" that would let you associate each labels layer with a given image layer... but for now, you basically have no choice but to have nC x 2 unassociated layers in napari if you have a mask for every image channel.

So for here, doing data[:, 0, :, :, :] will make sure you only have a single labels layer, but will obviously omit some data. So you could return a list of [(data[:,n,:,:,:],meta,'labels') for n in range(nChannels)] if you want to (and, until we provide layer groups, give them names that indicate which image layer they go with?)

lastly, singleton dimensions are handled well in napari, so if nC == 1, you wouldn't need to squeeze it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without that line, the masks shape is 5D: (40, 1, 31, 256, 256) whereas the Image channels have 4D shape (40, 31, 256, 256), so you get a mismatch of dimensions (masks and Image have independent T sliders):

Screenshot 2020-07-07 at 15 50 13

In 9f7f878 I've split mask channels into layers, as suggested by @tlambert03 - Thanks.

@will-moore
Copy link
Member Author

@joshmoore Your logic from ec13afd for detecting masks relies on ome/omero-cli-zarr#12 ?
I think I was missing that when writing the local logic.

@joshmoore
Copy link
Member

ome/omero-cli-zarr#12 ?

Yes. Happy to roll that back if you need to get this in before 12 lands.

@will-moore
Copy link
Member Author

This works now (loading masks):

$ napari https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001240.zarr

@joshmoore
Copy link
Member

Merging this as is. I'll updated ome/omero-ms-zarr#55 to list labeled images as zarrays rather than multiscale zgroups and then we can make that move in a follow up. (It'll be the first test of a breaking change!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants