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

Use zarrita #168

Merged
merged 4 commits into from
Nov 27, 2023
Merged

Use zarrita #168

merged 4 commits into from
Nov 27, 2023

Conversation

frasercl
Copy link
Collaborator

@frasercl frasercl commented Nov 22, 2023

Zarrita.js is a slightly more modular zarr library which works in a similar way to zarr.js, but fixes a number of problems we've been facing with the current library:

  • Most immediately relevant: errors caused by cancelled requests are properly caught (this seems to have been caused by zarr.js's use of p-queue for limiting concurrent requests; zarrita uses a dummy queue by default). This should remove the need for the hacky fix in Fix error when cancelling zarr requests #167
  • It allows us to pass down arbitrary options to its version of Store (called Readable), which solves a huge problem we've been facing with tracking prefetch requests

This PR switches us from zarr to zarrita. Remarkably little actually changes about how we interact with the library. The biggest change is converting the store wrapper from an instance of Store, which requires lots of methods that look like interacting with a filesystem, to an instance of Readable, which requires only one method: get.

@frasercl frasercl requested a review from a team as a code owner November 22, 2023 02:26
@frasercl frasercl requested review from toloudis, meganrm, blairlyons, interim17, ShrimpCryptid and ascibisz and removed request for a team November 22, 2023 02:26
@@ -213,8 +184,8 @@ function pickLevelToLoad(loadSpec: LoadSpec, spatialDimsZYX: [number, number, nu
return Math.max(optimalLevel, loadSpec.multiscaleLevel ?? 0);
}

function convertChannel(channelData: TypedArray, dtype: string): Uint8Array {
if (dtype === "|u1") {
function convertChannel(channelData: zarr.TypedArray<"uint8" | "uint16">): Uint8Array {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make this work with more types.

// TODO: fix "could not find a declaration file for module 'zarrita'." It has a .d.ts, so why isn't it being found?
// For now, I pinkie swear I'm using its types right
import * as zarr from "zarrita";
import { AbsolutePath, AsyncReadable, Readable } from "@zarrita/storage/dist/src/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there's any way to make TS happier that would be good

Base automatically changed from fix/better-slice-render to main November 27, 2023 19:01
@toloudis toloudis merged commit 0b3dc33 into main Nov 27, 2023
3 checks passed
@toloudis toloudis deleted the fix/zarrita branch November 27, 2023 19:01
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