N5 support #6466

leowe · 2022-09-13T17:21:26Z

adds support for N5
refactors data readers for Zarr and N5 so that they share a common architecture
extracts functionality into DatasetArray and DatasetHeader which ZarrArray, N5Array, ZarrHeader, and N5Header subclass and adapt the corresponding spec
the reading process of a N5 or Zarr dataset comprises three steps: (1) reading the dataset from file, (2) uncompressing it, and (3) building a multi-dimensional (in our case 3d) array in the right type from the uncompressed data
all 3 steps are done by separate components: (1) by FileSystemStore, (2) by Compressor, and (3) by MultiArray
the ChunkReader composes those components so that they are easier to swap out

URL of deployed dev instance (used for testing):

https://nsupport.webknossos.xyz

Steps to test:

Open any N5 dataset, e.g. jrc_cos7-11.n5.zip
NB: This dataset only uses values from 0-255

Issues:

fixes Support Reading N5 Datasets #6256

Updated (unreleased) changelog
Needs datastore update after deployment
Ready for review

TODO

Allow Compressor properties in attribute.json to also contain boolean values
Test dataset with data type different from uint8

…r in simpler composition instead of inheritance

# Conflicts: # project/Dependencies.scala

leowe · 2022-09-15T08:16:55Z

Open questions:

How to handle the datatypes? Currently the code just uses the ZarrDataType as the "universal" one? Should I introduce another one? That only feels like redundant information...
Should I also add a N5 dataset to the wk dev instance and to the Notion page?

fm3 · 2022-09-15T08:25:24Z

How to handle the datatypes? Currently the code just uses the ZarrDataType as the "universal" one? Should I introduce another one? That only feels like redundant information...

If the ZarrDataType also works for N5, maybe you can come up with a good name and place for it so that it can be used for both with good conscience.

Should I also add a N5 dataset to the wk dev instance and to the Notion page?

If that is relatively easy to do, I’d say yes :)

fm3

First scroll-through looks very promising :) I’ll review in more detail on monday, but the abstraction concept seems very nice!
Couple of naming suggestions:
FileSystemStoreN5 (and other things with N5 at the end) → N5FileSystemStore (I’d say it is nicer to have the most general at the end, this thing is a store, after all, not an n5)
resolvedDataType → dataType (is this a problem? you changed this for zarr)
chunkSize → chunkShape (size feels one-dimensional for me. now that we have this freedom, I’d go for shape, compare scalableminds/webknossos-libs#706)

normanrz · 2022-09-16T13:02:14Z

What would be missing for users to add remote N5 dataset (e.g. https://janelia-cosem-datasets.s3.amazonaws.com/jrc_macrophage-2/jrc_macrophage-2.n5) in webKnossos?

fm3 · 2022-09-16T13:14:21Z

At the very least the ExploreRemoteLayer functionality (where we detect ngff headers and create datasource-properties.jsons from them) needs to be extended to explore N5 datasets as well. That is not yet part of this PR. Edit: I wrote #6480 for that

normanrz · 2022-09-16T13:16:02Z

Should I also add a N5 dataset to the wk dev instance and to the Notion page?

If that is relatively easy to do, I’d say yes :)

Instead of putting the actual files on asterix, could also put a datasource-properties.json that references a remote dataset on asterix? Remote datasets are definitely more interesting to test than local ones.

fm3

Thanks for this great PR! I really like that you took the time to think about how this can be integrated with the existing zarr reading code.

I added some more comments in addition to my renaming requests above :)

Also I’m looking forward to test this with remote datasets, and with multiple mags. (Maybe you could prepare a datasource-properties.json, as Norman already suggested?)

webknossos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/DatasetPath.scala

...ssos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/jzarr/ZarrHeader.scala

webknossos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/n5/N5DataType.scala

webknossos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/n5/N5Header.scala

webknossos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/n5/N5DataType.scala

...ossos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/jzarr/ZarrArray.scala

webknossos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/Compressor.scala

...ssos-datastore/app/com/scalableminds/webknossos/datastore/datareaders/n5/ChunkReaderN5.scala

fm3 · 2022-09-19T11:10:43Z

...ossos-datastore/app/com/scalableminds/webknossos/datastore/dataformats/n5/N5DataLayers.scala

+  lazy val pathWithFallback: String =
+    path.getOrElse(if (mag.isIsotropic) s"${mag.x}" else s"${mag.x}-${mag.y}-${mag.z}")
+  private lazy val uri: URI = new URI(pathWithFallback)
+  private lazy val isRemote: Boolean = FileSystemsHolder.isSupportedRemoteScheme(uri.getScheme)
+  lazy val remoteSource: Option[RemoteSourceDescriptor] =
+    if (isRemote)
+      Some(RemoteSourceDescriptor(uri, credentials.map(_.user), credentials.flatMap(_.password)))
+    else
+      None


This reads a lot like ZarrMag. Maybe there can be a common trait? Not sure about a good name, though, ExtendedMag? Maybe you have a better Idea :D

This is exactly ZarrMag. I went ahead and used hte new class in both cases (DatasetLocatorMag)

If you have better ideas for the name, feel free to share ;)

…e/datareaders/n5/ChunkReaderN5.scala Co-authored-by: Florian M <[email protected]>

leowe · 2022-09-21T14:01:58Z

resolvedDataType → dataType (is this a problem? you changed this for zarr)

This is actually a problem because dataType is used by the N5 attribute.json and if we try to use it another way we will run into some overlap.

fm3

🎉

leowe and others added 2 commits August 26, 2022 14:08

add N5 layer classes

3d0f4ae

first working version

deb0602

leowe self-assigned this Sep 13, 2022

leowe added 3 commits September 14, 2022 19:19

refactor zarr and n5 with common superclasses, and refactor datareade…

3a05223

…r in simpler composition instead of inheritance

Merge branch 'master' into n5-support

7d02c62

# Conflicts: # project/Dependencies.scala

add PaddedChunkReader

4f55598

leowe requested a review from fm3 September 15, 2022 08:18

leowe added 3 commits September 15, 2022 10:25

rename zarrMagOpt to n5magopt

7e7b5f8

add CompressionOption

9ab0548

update changelog

e89d2ad

leowe marked this pull request as ready for review September 15, 2022 14:23

Merge branch 'master' into n5-support

f498de6

fm3 reviewed Sep 16, 2022

View reviewed changes

fm3 reviewed Sep 19, 2022

View reviewed changes

fm3 mentioned this pull request Sep 20, 2022

Explore/Import remote N5 datasets #6480

Closed

leowe and others added 3 commits September 20, 2022 18:33

Merge branch 'master' into n5-support

69ac5ec

Update webknossos-datastore/app/com/scalableminds/webknossos/datastor…

d379b89

…e/datareaders/n5/ChunkReaderN5.scala Co-authored-by: Florian M <[email protected]>

Merge branch 'master' into n5-support

c918707

leowe mentioned this pull request Sep 21, 2022

Unify DataType classes in wk #6484

Open

Merge branch 'master' into n5-support

7cb1d55

leowe added 2 commits September 21, 2022 16:22

incorporate pr feedback

a1dab94

reformat

8df6aeb

leowe requested a review from fm3 September 21, 2022 14:30

Merge branch 'master' into n5-support

94edba2

fm3 and others added 2 commits September 22, 2022 13:31

Fix JsonImplicits import, rename DatasetLocatorMag to MagLocator

a9fce98

Merge branch 'master' into n5-support

491515e

fm3 approved these changes Sep 22, 2022

View reviewed changes

Merge branch 'master' into n5-support

b0fc5ce

leowe merged commit 6d688e9 into master Sep 22, 2022

leowe deleted the n5-support branch September 22, 2022 13:31

frcroth mentioned this pull request Jun 2, 2023

Fix decompression of garbage data after valid gzip data causing decompression to fail #7119

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

N5 support #6466

N5 support #6466

leowe commented Sep 13, 2022 •

edited

Loading

leowe commented Sep 15, 2022

fm3 commented Sep 15, 2022

fm3 left a comment

normanrz commented Sep 16, 2022

fm3 commented Sep 16, 2022 •

edited

Loading

normanrz commented Sep 16, 2022

fm3 left a comment

fm3 Sep 19, 2022

leowe Sep 21, 2022

leowe Sep 21, 2022

leowe commented Sep 21, 2022 •

edited

Loading

fm3 left a comment

N5 support #6466

N5 support #6466

Conversation

leowe commented Sep 13, 2022 • edited Loading

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

leowe commented Sep 15, 2022

fm3 commented Sep 15, 2022

fm3 left a comment

Choose a reason for hiding this comment

normanrz commented Sep 16, 2022

fm3 commented Sep 16, 2022 • edited Loading

normanrz commented Sep 16, 2022

fm3 left a comment

Choose a reason for hiding this comment

fm3 Sep 19, 2022

Choose a reason for hiding this comment

leowe Sep 21, 2022

Choose a reason for hiding this comment

leowe Sep 21, 2022

Choose a reason for hiding this comment

leowe commented Sep 21, 2022 • edited Loading

fm3 left a comment

Choose a reason for hiding this comment

leowe commented Sep 13, 2022 •

edited

Loading

fm3 commented Sep 16, 2022 •

edited

Loading

leowe commented Sep 21, 2022 •

edited

Loading