feat(zarr): Flatten zarr module, remove XML-parsing #1462

manzt · 2021-06-07T14:46:47Z

Applies changes from #1446 to master

manzt · 2021-06-08T14:02:02Z

modules/zarr/src/lib/load-zarr.ts

  const tileSize = guessTileSize(data[0]);
+  const labels = options.labels ?? guessLabels(rootAttrs);


If no labels are provided, they are inferred (if possible) from the rootAttrs. For now, guessLabels only infers if OME-Zarr.

manzt · 2021-06-08T14:03:17Z

modules/zarr/src/lib/utils.ts

+  const grp = await openGroup(store, path);
+  const rootAttrs = await grp.attrs.asObject() as RootAttrs;
+
+  if (!Array.isArray(rootAttrs.multiscales)) {


checks that multiscales extension is implemented in rootAttrs (generic across Zarr datasets)

Is OME the only possible Zarr extension? Otherwise should we in the future make a more general interface for working with Zarr extensions?

A Zarr store is organized into a hierarchy of nodes. Each node can either be a group or an array, and only a group may contain other nodes. The multiscales extension is not OME-Zarr -specific (although it was spearheaded by OME folks working on zarr).

The mutiscales extension adds a "multiscales" key-value to group metadata that ultimately provides the relative paths to other array sub-resolutions saved in the store. OME-Zarr chooses to also store additional metadata in the same group but under a different key (omero). This function reads the multiscales metadata (to initialize the pixel-sources), but simply returns the complete root metadata ({ multiscales, omero } for Ome-Zarr) back to the end-user to choose what to do with it.

Sorry for the late addition, but there has been off and on discussion of moving the multiscales to a "super-interface" of OME-Zarr, i.e. a more generalized pyramidal image spec.

cf. pydata/xarray#4118 (comment)

manzt · 2021-06-08T14:05:07Z

modules/zarr/test/data/ome.zarr/.zattrs

+            "version": "0.1"
+        }
+    ],
+    "omero": {


OME metadata in root attrs. Used to distinguish multiscale dataset is OME-Zarr.

manzt · 2021-06-08T14:06:32Z

Should be ready for review. I consolidated the handling of different Zarr formats in loadZarr, and added an OME-Zarr fixture.

manzt · 2021-06-08T17:03:29Z

The tests are failing in the browser:

fetch-file.js:17 GET http://localhost:5000/modules/zarr/test/data/multiscale.zarr/.zgroup 404 (Not Found)

The issue seems to be with FetchFileStore which wraps fetchFile.

manzt · 2021-06-15T14:07:36Z

Rebased again on master. I commented out the browser tests for zarr due to the issue with fetchFile/FetchFileStore. I'm not sure how to resolve. It seems that a zarr store wrapping fetchFile should work in browser and node environments, but I'm not familiar enough with the implementation.

kylebarron · 2021-06-15T17:08:40Z

modules/zarr/package.json

@@ -1,6 +1,6 @@
 {
  "name": "@loaders.gl/zarr",
-  "version": "3.0.0-alpha.21",
+  "version": "3.0.0-alpha.18",


I think other modules are up to .21 now?

loaders.gl/modules/core/package.json

Line 3 in ba44020

"version": "3.0.0-alpha.21",

modules/zarr/package.json

modules/zarr/src/lib/load-zarr.ts

kylebarron · 2021-06-15T17:13:23Z

modules/zarr/src/lib/storage.ts

@@ -0,0 +1,38 @@
+import {fetchFile} from '@loaders.gl/core';


You removed @loaders.gl/core from the package.json...

kylebarron · 2021-06-15T17:14:00Z

modules/zarr/src/lib/storage.ts

+
+  async getItem(key: string, options?: RequestInit): Promise<ArrayBuffer> {
+    const filepath = joinUrlParts(this.root, key);
+    const response = await fetchFile(filepath, options);


Can you just use fetch here instead of fetchFile? For Node we expect users to import @loaders.gl/polyfills which polyfills fetch

The zarr.HTTPStore does exactly this ( uses fetch internally and expects a polyfil in node).

I guess the question is if loadZarr is given a string source, we need to pick the correct store to initialize FileFetchStore vs HTTPStore depending on the URL (one for a local store with node and the latter for a remote store), correct?

Still running into issues here. How can do I resolve the alias for a fixture path to either a path or URL? e.g.

const path = '@loaders.gl/zarr/test/data/multiscale.zarr'; // /Users/..../zarr/test/data/multiscale.zarr // or // https://localhost:5001/zarr/test/data/multiscale.zarr

Am I missing something?

This alias doesn't work?

loaders.gl/test/aliases.js

Line 61 in 24e3e60

'@loaders.gl/zarr/test': path.resolve(basename, '../modules/zarr/test'),

~~I'm missing something conceptually with how these aliases are resolved. For the following path:~~

const path = '@loaders.gl/zarr/test/data/multiscale.zarr';

How can I resolve this to the HTTP endpoint the file is served from (http://localhost:5000/modules/zarr/test/multiscale.zarr) for use with the fetch API? Calling resolvePath(path) returns /modules/zarr/test/multiscale.zarr)

Hmm, fetch('/modules/zarr/test/multiscale.zarr/.zgroup') responds with a 404 in the browser. I'm having trouble figuring out why this file isn't served.

kylebarron · 2021-06-15T17:16:08Z

modules/zarr/src/lib/utils.ts

+  const grp = await openGroup(store, path);
+  const rootAttrs = await grp.attrs.asObject() as RootAttrs;
+
+  if (!Array.isArray(rootAttrs.multiscales)) {


Is OME the only possible Zarr extension? Otherwise should we in the future make a more general interface for working with Zarr extensions?

manzt · 2021-06-24T18:41:17Z

There is still an issue with fetch-ing zarr files in the browser that I can't seem get to the bottom of (See comment above). This seems to be specific to fetching assets in the test suite, but the bundled modules do run in the browser.

I have disabled the browser tests, like geotiff, in favor of merging this PR sooner rather than later to avoid having this PR go stale. We can follow up in subsequent PR to fix both issues (geotiff + zarr).

manzt commented Jun 8, 2021

View reviewed changes

manzt requested a review from ibgreen June 8, 2021 14:05

manzt force-pushed the manzt/zarr-loader branch 2 times, most recently from dd9698d to 21094a6 Compare June 15, 2021 14:05

kylebarron approved these changes Jun 15, 2021

View reviewed changes

morphoix force-pushed the master branch from e7052bc to 53917b7 Compare June 21, 2021 09:27

belom88 force-pushed the master branch from 53917b7 to 2ab6e4a Compare June 21, 2021 09:46

manzt added 3 commits June 24, 2021 12:07

wip

dba4caf

Use DTYPE_LOOKUP for supported dtype types

6d281c8

Use store built on fetch API

e9d62e7

manzt force-pushed the manzt/zarr-loader branch from 2d5e6e6 to e9d62e7 Compare June 24, 2021 16:08

bump zarr

67fecb9

manzt force-pushed the manzt/zarr-loader branch from a09aca1 to 67fecb9 Compare June 24, 2021 17:55

manzt merged commit 6c5d201 into master Jun 24, 2021

manzt deleted the manzt/zarr-loader branch June 24, 2021 20:58

manzt mentioned this pull request Jul 6, 2021

resolution levels not a factor of 2 hms-dbmi/vizarr#101

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(zarr): Flatten zarr module, remove XML-parsing #1462

feat(zarr): Flatten zarr module, remove XML-parsing #1462

manzt commented Jun 7, 2021

manzt Jun 8, 2021

manzt Jun 8, 2021

kylebarron Jun 15, 2021

manzt Jun 15, 2021 •

edited

Loading

joshmoore Jul 9, 2021

manzt Jun 8, 2021

manzt commented Jun 8, 2021

manzt commented Jun 8, 2021

manzt commented Jun 15, 2021 •

edited

Loading

kylebarron Jun 15, 2021

kylebarron Jun 15, 2021

kylebarron Jun 15, 2021

manzt Jun 15, 2021 •

edited

Loading

manzt Jun 16, 2021

kylebarron Jun 16, 2021 •

edited

Loading

manzt Jun 16, 2021 •

edited

Loading

kylebarron Jun 15, 2021

manzt commented Jun 24, 2021 •

edited

Loading

		const tileSize = guessTileSize(data[0]);
		const labels = options.labels ?? guessLabels(rootAttrs);

feat(zarr): Flatten zarr module, remove XML-parsing #1462

feat(zarr): Flatten zarr module, remove XML-parsing #1462

Conversation

manzt commented Jun 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manzt Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manzt commented Jun 8, 2021

manzt commented Jun 8, 2021

manzt commented Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manzt Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylebarron Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

manzt Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manzt commented Jun 24, 2021 • edited Loading

manzt Jun 15, 2021 •

edited

Loading

manzt commented Jun 15, 2021 •

edited

Loading

manzt Jun 15, 2021 •

edited

Loading

kylebarron Jun 16, 2021 •

edited

Loading

manzt Jun 16, 2021 •

edited

Loading

manzt commented Jun 24, 2021 •

edited

Loading