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

BoolArray fails to get #118

Open
keller-mark opened this issue Sep 27, 2023 · 4 comments
Open

BoolArray fails to get #118

keller-mark opened this issue Sep 27, 2023 · 4 comments

Comments

@keller-mark
Copy link
Collaborator

https://observablehq.com/d/7152024fe1caf825 Using get() on a BoolArray seems to not work as expected - the returned value is { data: {}, shape: [34406], stride: [1] }

@keller-mark keller-mark added the bug Something isn't working label Sep 27, 2023
@keller-mark
Copy link
Collaborator Author

keller-mark commented Sep 27, 2023

Ah I see now the empty object is an iterator so I can do

Array.from((await zarr.get(arr)).data);

so perhaps not a bug after all, but now I have to always check for whether .data is an iterator on my end

const data = await zarr.get(arr);
if(data.data?.[Symbol.iterator]) {
  return Array.from(data.data);
}
return data.data;

@keller-mark
Copy link
Collaborator Author

Maybe the user should opt-in to getting an iterator

zarr.getIterator(arr)

@keller-mark keller-mark removed the bug Something isn't working label Sep 27, 2023
@manzt
Copy link
Owner

manzt commented Sep 27, 2023

Yeah, the idea was to keep the data in a TypedArray-like object for as long as possible (i.e., a strided view of the underlying bytes). But maybe this is more trouble than it's worth if there aren't use cases for keeping the underlying bytes.

This case happens with the string/bool array types, which is why I introduced the zarr.Array.is type guard. You could do something like:

if (arr.is("string") || arr.is("bool")) {
   data = Array.from((await get(arr)).data);
} else {
   data = (await get(arr)).data;
}

but maybe that's still not very ergonomic. We could probably wrap this in a separate API as you suggested, which will coerce the typed arrays into object arrays.

@manzt
Copy link
Owner

manzt commented Nov 3, 2023

A thought I had yesterday. Maybe we could have a type-aware helper for coercing the data types:

let { data } = zarr.refineChunk(await get(array), {
   string: ({ data, shape, stride }) => ({ data: Array.from(data), shape, stride }),
   boolean: ({ data, shape, stride }) => ({ data: Array.from(data), shape, stride }),
})

Will need to think about it more, but curious to hear your thoughts.

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

No branches or pull requests

2 participants