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

Fix/compressor #37

Merged
merged 11 commits into from
Mar 4, 2024
Merged

Fix/compressor #37

merged 11 commits into from
Mar 4, 2024

Conversation

cmhulbert
Copy link
Contributor

@cmhulbert cmhulbert commented Feb 21, 2024

  • fix a compressor serialization bug
  • add protected constructor to ZarrKeyValueReader to bypass root group exists check
  • refactor N5ZarrTest to support multiple backend testing in n5-universe [WIP]

cmhulbert and others added 11 commits February 21, 2024 16:38
When registering the ZarCompressor#Raw RawCompression with Gson via the HierarchyTypeAdapter, it would have two JsonSerializers for with id `"raw"`. The correct one was only being used based on order of the class loader finding the annotation. For this test, it would always load the `ZarrCompressor#Raw` variant first (which was the wrong one, as it resulted in an empty JsonObject, due to reflection) and the override it in he TypeAdapter map with the N5 RawCompression variant (which is the right one).

To ensure it doesn't rely on the order of the serializer, ZarrCompressor#Raw no longer extends RawCompression, and there is an explicit TypeAdapter for it now which Serializes to JsonNull.
This check was previously only in N5ZarrReader, but ideally those should be deprecated, so the logic should also belong in the ZarrKeyValueReader
…st class can be reused with different backends
@bogovicj bogovicj marked this pull request as ready for review March 4, 2024 23:43
@bogovicj bogovicj merged commit 0afb158 into saalfeldlab:master Mar 4, 2024
3 checks passed
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.

2 participants