issue_comments

This is just an internal check in xarray, verifying that _variables is always a Python dict. I would be fine turning this off, certainly at least for tests with the new custom mapping.

It seems I can make that check pass very easily, because this already works somehow, even though they aren't considered subtypes :confused:

```python In [1]: from xarray.core.utils import CopyableMutableMapping

In [2]: d = {}

In [3]: isinstance(d, CopyableMutableMapping) Out[3]: True ```

I realize that my assertion above isn't quite right — the issue was about Mapping[Hashable, V], not Dict[Hashable, V].

Changing CopyableMutableMapping[Hashable, Variable] to Mapping[Hashable, Variable] in the type hints for ._construct_direct and ._replace gets rid of loads of type errors! Must be because those methods don't actually need to mutate the mapping, so won't complain about .update if it's never called on those types.

Still have a bunch of errors like this though

xarray/core/dataset.py:785: error: Argument 1 to "Frozen" has incompatible type "CopyableMutableMapping[Hashable, Variable]"; expected "Mapping[Hashable, Variable]" [arg-type]

I realize that my assertion above isn't quite right — the issue was about Mapping[Hashable, V], not Dict[Hashable, V].

I'm thinking through this issue. I think the basic problem is that these CopyableMutableMapping aren't dict objects — given we can't inherit from dict (or Mapping) — and possibly we'd need to change our interface to CopyableMutableMapping for this to work. I'm not confident about this, though.

I think that with @runtime_checkable then once mypy considers these the same type then this would pass, but I might have completely misunderstood how that works (meaning then we would probably need to relax the isinstance check).

This is just an internal check in xarray, verifying that _variables is always a Python dict. I would be fine turning this off, certainly at least for tests with the new custom mapping.

I can have a look this week!

Thanks, but no rush! I write these comments just as much for my benefit as for anyone else's.

Would you mind committing the Hashable -> Any changes?

Sure, just done it in https://github.com/pydata/xarray/pull/6086/commits/1376ed9ffe756feefedd8e07e4068c7614ba39bb, though there might be other places that need it as a result.

I can have a look this week! Would you mind committing the Hashable -> Any changes? I'm confident that's the correct approach — we went through this at length on the main codebase — even if it doesn't fix all the probs...

I started adding the test implementation, but when I run the new test I get the exact isinstance error I was expecting:

``` ds = <xarray.Dataset> Dimensions: () Data variables: a int64 0 b int64 1

def _assert_dataset_invariants(ds: Dataset):

  assert isinstance(ds._variables, dict), type(ds._variables)

E AssertionError: <class 'xarray.tests.test_dataset.CustomMirroredMapping'>

/home/tom/Documents/Work/Code/xarray/xarray/testing.py:309: AssertionError `` I *think* that with@runtime_checkable` then once mypy considers these the same type then this would pass, but I might have completely misunderstood how that works (meaning then we would probably need to relax the isinstance check).

Thanks @max-sixty . Unfortunately changing Dict[Hashable, V] -> Dict[Any, V] and CopyableMutableMapping[Hashable, Variable] -> CopyableMutableMapping[Any, Variable] doesn't seem to help, I just get similar errors but with Any:

xarray/core/dataarray.py:538: error: Argument 1 to "_construct_direct" of "Dataset" has incompatible type "Dict[Any, Variable]"; expected "CopyableMutableMapping[Any, Variable]" [arg-type] xarray/core/dataarray.py:538: note: Following member(s) of "Dict[Any, Variable]" have conflicts: xarray/core/dataarray.py:538: note: Expected: xarray/core/dataarray.py:538: note: @overload xarray/core/dataarray.py:538: note: def update(self, other: Mapping[Any, Variable], **kwargs: Variable) -> None xarray/core/dataarray.py:538: note: @overload xarray/core/dataarray.py:538: note: def update(self, other: Iterable[Tuple[Any, Variable]], **kwargs: Variable) -> None xarray/core/dataarray.py:538: note: <1 more overload not shown> xarray/core/dataarray.py:538: note: Got: xarray/core/dataarray.py:538: note: @overload xarray/core/dataarray.py:538: note: def update(self, Mapping[Any, Variable], **kwargs: Variable) -> None xarray/core/dataarray.py:538: note: @overload xarray/core/dataarray.py:538: note: def update(self, Iterable[Tuple[Any, Variable]], **kwargs: Variable) -> None

To comment quickly (but incompletely atm) — Dict[Hashable, X] doesn't work well with mypy because Dict is invariant in its keys (I think) — it needs to be Dict[Any, X]