Conversation
petebankhead
left a comment
There was a problem hiding this comment.
This works for me, but I find it a bit confusing that the API is quite different for OMEZarrWriter and PyramidalOMEZarrWriter, even when it seems like they should be doing something similar.
For example,
OMEZarrWriter.Builderrequires downsamples to be passed in a method, whereasPyramidalOMEZarrWriterrequires them to be passed to the constructor; if they aren't needed at construction, then I think defaulting to a downsample of 1 would be fine.OMEZarrWriter.writeImagetakes no arguments, uses a background thread and returns immediately, whilePyramidalOMEZarrWriterrequires a path and is blocking.
Adding example scripts to the PR showing how the writers should be used would be good. I'd expect the scripts to look almost identical, so that it's easy to switch from one implementation to the other. If that's not possible, we'll also need to explain that clearly on ReadTheDocs.
I've added some comments on specific things.
...sion-bioformats/src/main/java/qupath/lib/images/writers/ome/zarr/PyramidalOMEZarrWriter.java
Outdated
Show resolved
Hide resolved
...-bioformats/src/test/java/qupath/lib/images/writers/ome/zarr/TestPyramidalOMEZarrWriter.java
Outdated
Show resolved
Hide resolved
...sion-bioformats/src/main/java/qupath/lib/images/writers/ome/zarr/PyramidalOMEZarrWriter.java
Outdated
Show resolved
Hide resolved
qupath-extension-bioformats/src/main/java/qupath/lib/images/writers/ome/zarr/WriterUtils.java
Outdated
Show resolved
Hide resolved
qupath-extension-bioformats/src/main/java/qupath/lib/images/writers/ome/zarr/WriterUtils.java
Outdated
Show resolved
Hide resolved
...sion-bioformats/src/main/java/qupath/lib/images/writers/ome/zarr/PyramidalOMEZarrWriter.java
Outdated
Show resolved
Hide resolved
...sion-bioformats/src/main/java/qupath/lib/images/writers/ome/zarr/PyramidalOMEZarrWriter.java
Outdated
Show resolved
Hide resolved
|
I addressed all comments. The public APIs of import qupath.lib.images.writers.ome.zarr.OMEZarrWriter
import qupath.lib.images.writers.ome.zarr.PyramidalOMEZarrWriter
var path = "/path/to/img.ome.zarr"
var server = getCurrentServer()
new OMEZarrWriter.Builder(server) // or new PyramidalOMEZarrWriter.Builder(server)
.downsamples(1, 4, 16)
.build(path)
.writeImage() |
|
It is now possible to provide downsamples without including 1. |
|
See also #2012 - it's a longstanding bug, and perhaps fixing it would be enough. |
|
I fixed a bug occurring when the first requested downsample isn't one. That doesn't fix your issue though. When I run your workflow, I also get a bit a CMU-1 in the output image. However, if I clear the tile cache between the two writes, then the output image is correct. So, I think this is clearly linked with #2012. How do you think we should fix that? Automatically clear the cache if two consecutive writes to the same location are detected? |
|
Please check out #2013 and see if that fixes the issue - I'm not sure how exactly the modified time behaves with directories. If you're temporarily creating a |
|
#2013 fixes the issue. |
|
I'll merge both then, thanks! |

Add a zarr writer that (in that order):
This provides less flexibility than the existing
OMEZarrWriterbut is less likely to throw out of memory errors.This PR only adds this new writer (
PyramidalOMEZarrWriter). Other changes to existing classes were made to reuse as much code as possible between the two zarr writers.I create this PR now to discuss about the public API (is only one
writeImage()function enough?), but it's a draft because tests need to be added.