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

Change dtype of ElementIdentifiers and DynamicTableRegion to int32 #85

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

Conversation

rly
Copy link
Contributor

@rly rly commented Nov 7, 2024

Summary of changes

Be explicit about the minimum integer precision required for ElementIdentifiers and DynamicTableRegion being int32. This is in anticipation of a change in the meaning of dtype: int to mean int8 instead of int32.

If the id columns of a DynamicTable (type: ElementIdentifiers) is initialized to be an int8 dataset and is set to be expandable, then you cannot add more than 128 rows to the table. You would have to delete the dataset and re-create it with a different dtype. Similarly, if a DynamicTableRegion is initialized to be an int8 dataset, referenced row indices could not be larger than 128. This is rare and may never come up, but is why @oruebel and I would prefer to keep the dtype of ElementIdentifiers and DynamicTableRegion to be int32 instead of int8.

See

PR checklist for schema changes

  • Update the version string in docs/source/conf.py and common/namespace.yaml to the next version with the suffix "-alpha"
  • Add a new section in the release notes for the new version with the date "Upcoming"
  • Add release notes for the PR to docs/source/hdmf_common_release_notes.rst and/or
    docs/source/hdmf_experimental_release_notes.rst

@rly rly requested review from bendichter and oruebel November 7, 2024 23:07
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.

1 participant