Refine chunks=None handling
#8249
Conversation
Based on comment in pydata#8247. This doesn't make it perfect, but allows the warning to get hit and clarifies the type comment, as a stop-gap
max-sixty
force-pushed
the
ds-chunks-none
branch
from
3f63f16 to
0d7c5cd
Compare
xarray/core/dataset.py
Outdated
| if not isinstance(chunks, Mapping) and chunks is not None: | ||
| # The ignore seems to be required because of the type change (though not | ||
| # completely sure, the message is somewhat confusing) | ||
| chunks = dict.fromkeys(self.dims, chunks) # type: ignore[arg-type] |
There was a problem hiding this comment.
Mypy doesn't like redefinitions. I've started to warm up to mypys opinion as well, it is annoying when a variable changes behavior significantly after a small if-check.
Any time a variable changes type you should expect to create a new variable, so try something like this:
chunks_dict: dict
if not isinstance(chunks, Mapping) and chunks is not None:
chunks_dict = dict.fromkeys(self.dims, chunks)Remember the downstream names as well.
There was a problem hiding this comment.
I agree! Though in this case, that didn't seem to completely satisfy mypy either. I think pushing until we remove the deprecation is probably reasonable...
There was a problem hiding this comment.
Aha, the error was about the .fromkeys second input. Renaming still seems to work, the typing for chunks_mapping can probably be improved.
Sidenote but I'm quite surprised this worked:
dict.fromkeys((3,4,5), "auto")
Out[9]: {3: 'auto', 4: 'auto', 5: 'auto'}
# I was expecting:
{3: 'a', 4: 'u', 5: 't'}There was a problem hiding this comment.
OK nice! I hadn't thought to make it Mapping[Any, Any], I agree it's better now even if not perfect. Thanks!
Based on comment in #8247. This doesn't make it perfect, but allows the warning to get hit and clarifies the type comment, as a stop-gap