-
Notifications
You must be signed in to change notification settings - Fork 5
Warn about Zarr layouts that make viewing awkward #430
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { getDatasetWarnings } from '@/omezarr-helper'; | ||
| import type { Metadata } from '@/omezarr-helper'; | ||
|
|
||
| // Minimal stand-in for the parts of Metadata the checks read. Codec info is | ||
| // left out by default, which the chunk check treats as compressed. | ||
| const createMetadata = ( | ||
| chunks: number[], | ||
| dtype = 'uint16', | ||
| extra: Partial<Metadata> = {}, | ||
| shape: number[] = [8, 8, 8] | ||
| ): Metadata => | ||
| ({ | ||
| arr: { chunks, dtype, shape }, | ||
| ...extra | ||
| }) as unknown as Metadata; | ||
|
|
||
| const axes = (...names: string[]): Partial<Metadata> => ({ | ||
| multiscales: [ | ||
| { axes: names.map(name => ({ name, type: 'space' })), datasets: [{}, {}] } | ||
| ] as unknown as Metadata['multiscales'] | ||
| }); | ||
|
|
||
| const levels = (count: number): Partial<Metadata> => ({ | ||
| multiscales: [ | ||
| { datasets: Array.from({ length: count }, () => ({})) } | ||
| ] as unknown as Metadata['multiscales'] | ||
| }); | ||
|
|
||
| // zstd nested inside a sharding_indexed pipeline, as a sharded v3 array stores it. | ||
| const SHARDED_ZSTD: Partial<Metadata> = { | ||
| codecs: [ | ||
| { | ||
| name: 'sharding_indexed', | ||
| configuration: { codecs: [{ name: 'bytes' }, { name: 'zstd' }] } | ||
| } | ||
| ] | ||
| }; | ||
| const UNCOMPRESSED_V3: Partial<Metadata> = { | ||
| codecs: [{ name: 'bytes' }, { name: 'crc32c' }] | ||
| }; | ||
|
|
||
| describe('getDatasetWarnings: chunk size', () => { | ||
| it('says nothing about reasonable chunks', () => { | ||
| expect(getDatasetWarnings(createMetadata([64, 64, 64]))).toEqual([]); | ||
| }); | ||
|
|
||
| it('does not warn about a compressed 16 MB chunk', () => { | ||
| // raw/s2: 16 MB inner chunks that zstd takes to ~12 MB on disk. | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([8, 128, 128, 128], 'uint8', SHARDED_ZSTD) | ||
| ) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('holds an uncompressed array to the stricter limit', () => { | ||
| // The same 16 MB chunks, but stored raw, so 16 MB is what transfers. | ||
| for (const raw of [UNCOMPRESSED_V3, { compressor: null }]) { | ||
| expect( | ||
| getDatasetWarnings(createMetadata([8, 128, 128, 128], 'uint8', raw)) | ||
| ).toEqual([ | ||
| { case: 'zarr-large-chunks', size: '16 MB', compressed: false } | ||
| ]); | ||
| } | ||
| }); | ||
|
|
||
| it('finds a compressor nested inside a sharding codec', () => { | ||
| // sharding_indexed is structural, so a flat scan would call this | ||
| // uncompressed and warn at 16 MB. | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([8, 128, 128, 128], 'uint8', SHARDED_ZSTD) | ||
| ) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('assumes compressed when codec metadata was never fetched', () => { | ||
| // Unknown lands on the permissive limit: a missed warning beats a false one. | ||
| expect( | ||
| getDatasetWarnings(createMetadata([8, 128, 128, 128], 'uint8')) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('warns above the compressed limit', () => { | ||
| // seed151 img: 128 MB chunks. | ||
| expect( | ||
| getDatasetWarnings(createMetadata([256, 256, 256, 8], 'uint8')) | ||
| ).toEqual([ | ||
| { case: 'zarr-large-chunks', size: '128 MB', compressed: true } | ||
| ]); | ||
| }); | ||
|
|
||
| it('accounts for the dtype width', () => { | ||
| expect(getDatasetWarnings(createMetadata([256, 256, 256]))).toEqual([]); | ||
| expect( | ||
| getDatasetWarnings(createMetadata([256, 256, 256], 'float64')) | ||
| ).toHaveLength(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getDatasetWarnings: resolution levels', () => { | ||
| const BIG = [3000, 3000, 1350, 8]; // 91 GB of uint8, the seed151 img extent | ||
|
|
||
| it('warns when multiscales declares a single level for a large image', () => { | ||
| expect( | ||
| getDatasetWarnings(createMetadata([64, 64, 64], 'uint8', levels(1), BIG)) | ||
| ).toEqual([{ case: 'zarr-single-level', size: '91 GB' }]); | ||
| }); | ||
|
|
||
| it('says nothing when the pyramid has levels', () => { | ||
| expect( | ||
| getDatasetWarnings(createMetadata([64, 64, 64], 'uint8', levels(5), BIG)) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('says nothing about a small single-level image', () => { | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([64, 64, 64], 'uint8', levels(1), [256, 256, 256]) | ||
| ) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('never fires on a plain array, however large', () => { | ||
| // The bug that made this warn on raw/s2: a plain array also has one shape, | ||
| // but it declares no multiscales and so claims nothing. | ||
| expect( | ||
| getDatasetWarnings(createMetadata([64, 64, 64], 'uint8', {}, BIG)) | ||
| ).toEqual([]); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getDatasetWarnings: axis order', () => { | ||
| it('accepts spec order', () => { | ||
| for (const names of [ | ||
| ['t', 'c', 'z', 'y', 'x'], | ||
| ['c', 'z', 'y', 'x'], | ||
| ['z', 'y', 'x'], | ||
| ['y', 'x'] | ||
| ]) { | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([64, 64, 64], 'uint16', axes(...names)) | ||
| ) | ||
| ).toEqual([]); | ||
| } | ||
| }); | ||
|
|
||
| it('warns about the seed151 c,x,y,z order', () => { | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([64, 64, 64, 8], 'uint8', axes('c', 'x', 'y', 'z')) | ||
| ) | ||
| ).toEqual([ | ||
| { | ||
| case: 'zarr-axis-order', | ||
| axisOrder: 'C, X, Y, Z', | ||
| expectedOrder: 'C, Z, Y, X' | ||
| } | ||
| ]); | ||
| }); | ||
|
|
||
| it('warns when the channel axis trails the spatial axes', () => { | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([64, 64, 64, 8], 'uint8', axes('x', 'y', 'z', 'c')) | ||
| ) | ||
| ).toEqual([ | ||
| { | ||
| case: 'zarr-axis-order', | ||
| axisOrder: 'X, Y, Z, C', | ||
| expectedOrder: 'C, Z, Y, X' | ||
| } | ||
| ]); | ||
| }); | ||
|
|
||
| it('is case insensitive', () => { | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([64, 64, 64], 'uint16', axes('Z', 'Y', 'X')) | ||
| ) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('stays quiet about custom axes it cannot judge', () => { | ||
| expect( | ||
| getDatasetWarnings( | ||
| createMetadata([64, 64, 64], 'uint16', axes('c', 'angle', 'y', 'x')) | ||
| ) | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('stays quiet about a plain array with no axes', () => { | ||
| expect(getDatasetWarnings(createMetadata([64, 64, 64]))).toEqual([]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,10 @@ type MetadataHintVariant = | |
| | { case: 'zarr-v2-no-multiscales' } | ||
| | { case: 'zarr-v3-no-multiscales' } | ||
| | { case: 'zarr-query-error'; errorMessage?: string } | ||
| // Zarr - metadata is valid, but the layout will make viewing awkward | ||
| | { case: 'zarr-single-level'; size: string } | ||
| | { case: 'zarr-large-chunks'; size: string; compressed: boolean } | ||
| | { case: 'zarr-axis-order'; axisOrder: string; expectedOrder: string } | ||
| // N5 - query never fired | ||
| | { case: 'n5-has-s0-no-attrs' } | ||
| | { case: 'n5-has-attrs-no-s0' } | ||
|
|
@@ -72,6 +76,24 @@ function getHintConfig(variant: MetadataHintVariant): HintConfig { | |
| ? `Could not read Zarr metadata. ${variant.errorMessage}` | ||
| : 'Could not read Zarr metadata.' | ||
| }; | ||
| case 'zarr-single-level': | ||
| return { | ||
| kind: 'warning', | ||
| title: 'Only one resolution level', | ||
| description: `This dataset declares multiscales but only supplies a single level, for ${variant.size} of data. Viewers must read full-resolution at every zoom level, so viewing the whole image is far more expensive than it needs to be. Generating a multiscale pyramid fixes this.` | ||
| }; | ||
| case 'zarr-axis-order': | ||
| return { | ||
| kind: 'warning', | ||
| title: 'Axes are not in the order OME-Zarr specifies', | ||
| description: `The axes are ordered ${variant.axisOrder}, but the spec requires T, C, Z, Y, X order - here that would be ${variant.expectedOrder}. Many tools take the last two axes to be the image plane, so they will show a cross-section rather than the expected view. Rewriting the dataset with the axes in spec order avoids this.` | ||
| }; | ||
| case 'zarr-large-chunks': | ||
| return { | ||
| kind: 'warning', | ||
| title: 'Chunks may be too large for efficient viewing', | ||
| description: `This dataset uses ${variant.size} chunks ${variant.compressed ? '(before compression)' : '(without compression)'}. Chunk files larger than the browser's cache limit are re-downloaded on every access, which makes viewing slow. A final chunk size of 1-10 MB works best.` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may need to qualify this for shards. Shards are also large top-level chunks.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I follow. Shards are on the server side, the client side deals with chunks. Why would shard size matter to the client? |
||
| }; | ||
| case 'n5-has-s0-no-attrs': | ||
| logger.info( | ||
| 'This folder has a .n5 extension but does not contain an attributes.json file required for N5 metadata preview.' | ||
|
|
||
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.
RFC 3 will likely make this advice obsolete with respect to the spec statement:
https://ngff.openmicroscopy.org/rfc/3/
Uh oh!
There was an error while loading. Please reload this page.
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.
I know that, but in the meantime it is creating issues, specifically with ome-zarr.js (and I would bet there are other cases).
For example: https://github.com/BioNGFF/ome-zarr.js/blob/main/src/image.ts#L336-L337