-
Notifications
You must be signed in to change notification settings - Fork 35
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
Update to new jzarr 0.4.2 #203
Conversation
This is the latest release based on https://github.com/zarr-developers/jzarr.
dev.jzarr:jzarr is in Maven central.
These are probably the most relevant missing changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following a discussion with @melissalinkert, a few comments:
- the
com.bc.zarr:jzarr
library should be considered as inactive - the cleanup of all intermediate repositories and the ability to use Maven Central is really nice
- looking nature of the changes between the 0.3.5 and the 0.4.0 changes bcdev/jzarr@0.3.5...zarr-developers:jzarr:0.4.0, I see nothing of particular concerns:
- essential metadata got added to the POM like the license 👍
- a few API additions which could be consumed by the converter e.g. the multi-threaded blosc compression
- the bump of the jackson dependency which is overriden by a more recent versions from
ome:formats-gpl
anyways
- at present, https://github.com/zarr-developers/jzarr feels like the best place to discuss & propose new API additions which would be relevant to the needs of this library
- we will likely need to unify the change across all conversion utilities to avoid a mixture of
jzarr
Could we update
bioformats2raw/src/test/java/com/glencoesoftware/bioformats2raw/test/ZarrTest.java
Lines 270 to 273 in b7c5440
// no getter for DimensionSeparator in ZarrArray | |
// check that the correct separator was used by checking | |
// that the expected first chunk file exists | |
assertTrue(output.resolve("0/0/0/0/0/0/0").toFile().exists()); |
👍 maybe let's just remove the outdated test comment and then this should be ready
Next question is whether we want to plan a 0.7.0 release or a release candidate that can be consumed in all the related converter utility PRs adjusting the dependency to use |
Test comment updated in 609f092. As discussed separately, this PR should not be merged right now, pending investigation of zarr-developers/jzarr#14. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retested the last commit and confirmed that:
- the array metadata of the generated Zarr datasets no longer include the additional
numThreads
key - the Zarr datasets can be opened both using the
zarr-python
implementation and another Java implementation e.g.raw2ometiff
A few recent maintenance issues (zarr-developers/jzarr#17) are further reinforcing my feeling expressed in #203 (review) i.e. the inactive state of com.bc.zarr:jzarr
effectively elevates the dev.zarr:jzarr
fork as the new reference Java implementation for the Zarr v2 specification.
Prompted by ome/ZarrReader#54, see https://github.com/zarr-developers/jzarr.
Discussed briefly with @sbesson. I am not suggesting that we consider this for bioformats2raw 0.7.0, it's more a place to start. Whatever we decide to do here should likely be applied to other repos where we use jzarr (including but not limited to raw2ometiff).