issue_comments

thanks @crusaderky !

@shoyer implemented all change requests except OrderedDict

@shoyer I changed dimensions from str to Hashable as requested. In my opinion however it's not great, because

1. it's currently thoroughly broken due to lack of unit tests
2. it weakens type checking a lot, since Tuple is a subclass of Hashable however the vast majority of xarray functions deal with tuples with a specific code branch

I agree it's not great. But some of our APIs are going to be hard to type check regardless, e.g., in cases where we have different behavior for strings vs sequences of strings.

@shoyer I changed dimensions from str to Hashable as requested. In my opinion however it's not great, because 1. it's currently thoroughly broken due to lack of unit tests 2. it weakens type checking a lot, since Tuple is a subclass of Hashable however the vast majority of xarray functions deal with tuples with a specific code branch

Please review again - in theory, common.py and util.py are now 100% done. They pass flake8 and mypy (on python 3.7). If you like them please go on and merge into Master; since the work that needs to be done is going to touch literally everything I think it's best to break it down into more manageable chunks.

If it's not too much trouble, I'd like to keep the xarray (with type checking) working on Python 3.5.0 for now. The type checks themselves only need to work on Python 3.7.

This should be relatively straightforward if you put type annotations in strings as needed and guard typing imports in if TYPE_CHECKING blocks.

FYI we added some code here to deal with python 3.5.0-3.5.2: https://github.com/pydata/xarray/pull/2831

Opened https://bugs.python.org/issue36555

@shoyer I have a problem. In Python 3.5.0-3.5.2, to create a custom typed collection you have to write class Frozen(collections.abc.Mapping, typing.Mapping[K, V]): Whereas in Python >= 3.5.3, you must just write class Frozen(typing.Mapping[K, V]): Things won't work if you use the < 3.5.3 syntax.

As an unrelated note, I'm observing that if I set python=3.5.3 in ci/requirements-py35.yml, a ton of unit tests fall apart. The test suite ci/requirements-py35-min.yml ensures compatibility with Python 3.5.0... but only as long as you don't use any of the non-mandatory dependencies. Can we 1) increase the minimum requirements to 3.5.4 and 2) set python=3.5.4 in ci/requirements-py35.yml? Is there a roadmap for dropping 3.5 entirely? (conda-forge already did...)

Mmh it looks like if I run mypy in Python 3.7 I get a much terser output. Still some horrible broken things to care about, like the fact that in 3.5 typing.Mapping did not inherit from collections.abc.Mapping, but they can be worked around somehow.

@shoyer I am using mypy. But unlike flake8 I don't think I'll be able to integrate it in CI. This error doesn't make any sense to me: xarray/core/common.py:124: error: Overloaded function signatures 1 and 2 overlap with incompatible return types

These I think are unfixable in Python 3.5, as I can't declare these under the class definition: xarray/core/utils.py:445: error: "NdimSizeLenMixin" has no attribute "shape" xarray/core/utils.py:450: error: "NdimSizeLenMixin" has no attribute "shape" xarray/core/utils.py:454: error: "NdimSizeLenMixin" has no attribute "shape" xarray/core/utils.py:468: error: "NDArrayMixin" has no attribute "array" xarray/core/utils.py:472: error: "NDArrayMixin" has no attribute "array" xarray/core/utils.py:478: error: "NDArrayMixin" has no attribute "array" xarray/core/common.py:151: error: "AbstractArray" has no attribute "dims" xarray/core/common.py:154: error: "AbstractArray" has no attribute "dims" xarray/core/common.py:166: error: "AbstractArray" has no attribute "dims" xarray/core/common.py:166: error: "AbstractArray" has no attribute "shape" xarray/core/common.py:896: error: Cannot determine type of '_file_obj' xarray/core/common.py:897: error: Cannot determine type of '_file_obj' (additionally, typing doesn't offer anything for "file-like" objects)

From the 3.5.2 release notes:

A new version of typing.py provides several new classes and features: @overload outside stubs, Reversible, DefaultDict, Text, ContextManager, Type[], NewType(), TYPE_CHECKING, and numerous bug fixes (note that some of the new features are not yet implemented in mypy or other static analyzers). Also classes for PEP 492 (Awaitable, AsyncIterable, AsyncIterator) have been added (in fact they made it into 3.5.1 but were never mentioned).

@shoyer how do you feel about raising the minimum Python version to 3.5.2? There's quite a bit of important stuff in it, some of which undocumented. Namely @overload doesn't work with 3.5.0 (although I could conditionally replace it with a no-op thorough a backport module).