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

Zarr reading and writing support #1474

Merged
merged 21 commits into from Apr 30, 2024
Merged

Zarr reading and writing support #1474

merged 21 commits into from Apr 30, 2024

Conversation

Rylern
Copy link
Contributor

@Rylern Rylern commented Feb 23, 2024

This PR adds the possibility to open Zarr images.

This can be done by clicking on File->Open URI..., and writing the path to the Zarr directory. Dragging the directory to QuPath or using File->Open doesn't work. Should I improve this?

@petebankhead petebankhead added this to the v0.6.0 milestone Feb 26, 2024
@alanocallaghan
Copy link
Contributor

Let me know when this is tested on different platforms so I can do a non-snapshot blosc release

@Rylern
Copy link
Contributor Author

Rylern commented Feb 27, 2024

It works on Linux, Windows, and Mac with Apple Silicon. So the only remaining OS is Mac on Intel

@petebankhead
Copy link
Member

This is huge! I should be able to try it on an Intel Mac - is there a link to any good test image?

@Rylern
Copy link
Contributor Author

Rylern commented Feb 29, 2024

@petebankhead
Copy link
Member

petebankhead commented Feb 29, 2024

Works for me on an Intel Mac!

This can be done by clicking on File->Open URI..., and writing the path to the Zarr directory. Dragging the directory to QuPath or using File->Open doesn't work. Should I improve this?

Yeah, we'll need to provide a nicer way to open the images or add to a project - ideally including a drag & drop option.

Will take a bit of thought to figure out how to do it. Maybe we need an 'Open image directory' option? But then people might assume that it opens all the images in a directory (which, I suppose, it something it could do...).

DICOM is also a more recently-relevant format that our current 'open / add' mechanism doesn't support well: #1397

So we should probably think about both together - probably as a separate PR.

@Rylern
Copy link
Contributor Author

Rylern commented Feb 29, 2024

About commit a7b184f:

In Zarr files (or at least in the Zarr files I have), the z values are scaled with the resolution. This means that if there is an image of shape (t: 10, c: 20, z: 30, y:40, x: 50) at full resolution, it will have a shape of (t: 10, c: 20, z: 15, y:20, x: 25) with a downsample of 2 (and not (t: 10, c: 20, z: 30, y:20, x: 25)).

This means that in the BioFormats IFormatReader.getIndex(z, c, t) function, z can only take values between 0 and 15 (and not 30) when called for the image of downsample 2.

I'm not satisfied with this commit because it's ugly and assumes every Zarr files scale the z values with the resolution (which I was not able to find if it's the case in the OME-ZARR specs).

@petebankhead
Copy link
Member

Ah, yes that does seem to be a significant complication... have you seen this? ome/bioformats#3794

If we were able to support 2D pyramidal Zarr in QuPath v0.6.0 that would already be a major step forward (especially if we can write as well) - and we could delay worrying about multiresolution z until we switch to ImgLib2 and (most likely) an entirely new viewer.

@alanocallaghan
Copy link
Contributor

Great, shall I push out a proper release of the blosc jar, then?

Will take a bit of thought to figure out how to do it. Maybe we need an 'Open image directory' option? But then people might assume that it opens all the images in a directory (which, I suppose, it something it could do...).

Both useful, but should be distinguished semantically in some way

@Rylern
Copy link
Contributor Author

Rylern commented Feb 29, 2024

If we were able to support 2D pyramidal Zarr in QuPath v0.6.0 that would already be a major step forward (especially if we can write as well) - and we could delay worrying about multiresolution z until we switch to ImgLib2 and (most likely) an entirely new viewer.

OK, so should I remove the last commit?

Both useful, but should be distinguished semantically in some way

I may be missing something obvious, but can't we just allow the File->Open window to open files AND directories?

@alanocallaghan
Copy link
Contributor

I may be missing something obvious, but can't we just allow the File->Open window to open files AND directories?

My feeling is that mixing the two behaviours could lead to weird or unexpected behaviour if eg I open a directory that contains zarr(s)

@petebankhead
Copy link
Member

I think in JavaFX we can only have a file or a directory chooser... and you need to select the right chooser since they can't be mixed (at least on macOS - it might be platform-dependent).

Haven't checked out the last commit yet...

@Rylern
Copy link
Contributor Author

Rylern commented Mar 1, 2024

I improved the z-stack handling as we discussed yesterday @petebankhead


// Some files provide z scaling (the number of z stacks decreases when the resolution becomes
// lower, like the width and height), so z needs to be updated for levels > 0
if (level > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if ipReader.setResolution is expensive. In case it is, we should try to avoid calling it.

  • Here, I think you can put this block before the initial ipReader.setResolution(level) and include that in an else (since this block set the resolution to the level at the end anyway).
  • You can also use if (level > 0 && z > 0) since otherwise it shouldn't matter

ipReader.setResolution(level);
int zStacksCurrentResolution = ipReader.getSizeZ();

if (zStacksFullResolution != zStacksCurrentResolution) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be round or floor? My intuition says round but my intuition isn't certain...

Also, it would be useful to look at the number of z slices for different levels in the sample images you have access to. It might be possible to compute the ratio between the z slices for a level and the zero level, to determine how much to rescale the z index (which might be more robust than using the downsample value, in case it's possible that the downsampling in z uses a different factor).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to use round but I got an error when z / tileRequest.getDownsample() was giving something like 133.5, which turned into 134 after applying round, when the max possible value was 133.

@Rylern
Copy link
Contributor Author

Rylern commented Mar 1, 2024

I addressed the two issues

@Rylern Rylern changed the title Zarr reading support Zarr reading and writing support Mar 4, 2024
@petebankhead petebankhead merged commit b4a4c15 into qupath:main Apr 30, 2024
3 checks passed
@Rylern Rylern deleted the read-zarr branch April 30, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants