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

ENH: Add data type to sparse #783

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

genematx
Copy link
Contributor

@genematx genematx commented Sep 6, 2024

Unlike the (dense) ArrayAdapter, adapters for sparse arrays (e.g. COOAdapter) didn't have dtype explicitly stored. This PR adds a new property, data_type, to COOAdapter and the corresponding COOStructure; the new property is initialized similarly to how it's done for ArrayAdapter, including the possibility to use StructDtypes.

The changes are caused by the requirement to specify dtype when exposing the arrays at /zarr/v2 endpoints (see Issue #562 and PR #774).

Checklist

  • Add a Changelog entry
  • Add the ticket number which this PR closes to the comment section

Copy link
Member

@danielballan danielballan left a comment

Choose a reason for hiding this comment

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

I had to think about how this ever worked in the first place! Since we move the data in Arrow IPC format by default, the data types come free. Clearly you are correct: we should be writing down the data type.

Let's merge after some third person has chance to kick the tires and confirm interactively that there are no surprises here.

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