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

Support non-str Hashables in DataArray #8559

Merged
merged 10 commits into from
Jan 14, 2024

Conversation

headtr1ck
Copy link
Collaborator

@headtr1ck headtr1ck commented Dec 18, 2023

Probably we should add a whole bunch of tests for this. For now only testing the constructor.

xarray/core/dataarray.py Outdated Show resolved Hide resolved
@TomNicholas
Copy link
Member

Probably we should add a whole bunch of tests for this. For now only testing the constructor.

I think I've said this before but eventually I would like to see us use property-based testing and test using a strategy that generates arbitrary strings / hashables. We could build off of #6903 and generalise the names strategy that was introduced in #8404, for example.

@max-sixty
Copy link
Collaborator

max-sixty commented Dec 18, 2023

Probably we should add a whole bunch of tests for this.

We could add them onto a canonical test object like #8533 (maybe there's a param for "object with lots of quirks"), and then if we can spread that object throughout the test suite, we get every method tested on the full range of quirks


(I see @TomNicholas just replied with a suggestion to use hypothesis — agree — I think that would provide more depth (more quirks!), though less wide coverage, given we're unlikely to write every test using hypothesis)

@headtr1ck
Copy link
Collaborator Author

Should we explicitly disallow using None as dimension/variable names?
It will inadvertently lead to problems later on.

@headtr1ck headtr1ck added the plan to merge Final call for comments label Dec 20, 2023
@headtr1ck headtr1ck enabled auto-merge (squash) January 14, 2024 20:19
@headtr1ck headtr1ck merged commit 357a444 into pydata:main Jan 14, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataArray types must be strings
3 participants