Use Self rather than concrete types, remove casts
#8216
Conversation
This should also allow for subtyping
for more information, see https://pre-commit.ci
max-sixty
force-pushed
the
typing-self
branch
from
0e38502
to
29084a7
Compare
max-sixty
force-pushed
the
typing-self
branch
from
b667732
to
155ab5d
Compare
max-sixty
force-pushed
the
typing-self
branch
from
1aa3286
to
106c8ce
Compare
xarray/core/dataset.py
Outdated
| return _broadcast_helper( | ||
| cast("T_Dataset", args[1]), exclude, dims_map, common_coords | ||
| cast("Dataset", args[1]), exclude, dims_map, common_coords |
There was a problem hiding this comment.
I guess we have to look at what align does to subclasses
There was a problem hiding this comment.
Yes, I looked at this, but eventually thought it wasn't worth gating on — it's quite complicated. It's something like "the 'biggest' class" — so "da + da = da", but "da + ds = ds"...
There was a problem hiding this comment.
I adjusted these a bit, see what you think. Allowed removing some casts
Co-authored-by: Michael Niklas <[email protected]>
Co-authored-by: Michael Niklas <[email protected]>
Co-authored-by: Michael Niklas <[email protected]>
for more information, see https://pre-commit.ci
xarray/core/coordinates.py
Outdated
| @@ -664,6 +664,8 @@ def copy( | |||
| variables = { | |||
| k: v._copy(deep=deep, memo=memo) for k, v in self.variables.items() | |||
| } | |||
| # Currently this requires using the concrete type of `Coordinates`, rather than | |||
| # `Self`, ref #8216 | |||
There was a problem hiding this comment.
why doesn't type(self) work here?
return type(self)._construct_direct(
coords=variables, indexes=dict(self.xindexes), dims=dict(self.sizes)
)There was a problem hiding this comment.
I went to answer this question, and then got it working :)
There was a problem hiding this comment.
OK actually the typing worked but the tests didn't; switching back. Added a TODO but won't chase it down atm...
This should also allow for subtyping