Skip to content

Conversation

@TomNicholas
Copy link
Member

Rectifies the the issue in #5843 by making DataArray.chunks and Variable.chunks consistent with Dataset.chunks. This would obviously need a deprecation cycle before it were merged.

Currently a WIP - I changed the behaviour but this obviously broke quite a few tests and I haven't looked at them yet.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2021

Unit Test Results

         6 files           6 suites   55m 45s ⏱️
16 226 tests 14 433 ✔️ 1 736 💤   57
90 552 runs  82 030 ✔️ 8 180 💤 342

For more details on these failures, see this check.

Results for commit 690bde8.

def chunks(self) -> Optional[Tuple[Tuple[int, ...], ...]]:
"""Block dimensions for this array's data or None if it's not a dask
array.
def chunks(self) -> Optional[Mapping[Hashable, Tuple[int, ...]]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def chunks(self) -> Optional[Mapping[Hashable, Tuple[int, ...]]]:
def chunks(self) -> Optional[Mapping[Any, Tuple[int, ...]]]:

We switched all these because of how mypy handles variance of key types

@max-sixty
Copy link
Collaborator

Code looks good.

I would generally weigh towards breaking changes if it improves consistency, but realize this is probably a close one. One synthesis of those is a new consistent method and we deprecate the old one. But not sure if there's a comparably good name than .chunks...

@TomNicholas TomNicholas mentioned this pull request Oct 26, 2021
5 tasks
@TomNicholas
Copy link
Member Author

Superceded by #5900

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.

Why are da.chunks and ds.chunks properties inconsistent?

2 participants