Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add typing-extensions to the list of dependencies? #5495

Closed
malmans2 opened this issue Jun 19, 2021 · 8 comments · Fixed by #5503 or #5624
Closed

Add typing-extensions to the list of dependencies? #5495

malmans2 opened this issue Jun 19, 2021 · 8 comments · Fixed by #5503 or #5624

Comments

@malmans2
Copy link
Contributor

typing_extensions is imported in xarray/core/npcompat.py and xarray/core/utils.py.

However, typing-extensions is not a dependency in setup.cfg, and I get the following error when I install with pip install git+https://github.com/pydata/xarray using python 3.9:

import xarray

ModuleNotFoundError: No module named 'typing_extensions'

@max-sixty
Copy link
Collaborator

Great spot! We should definitely add that.

I wonder how CI passes, then?

@malmans2
Copy link
Contributor Author

Looks like there isn't an action that only installs xarray using pip, but dependencies are installed first using conda. Must be in the conda recipe of one of the packages specified in the CI environments?

@max-sixty
Copy link
Collaborator

Yes, good point.

Would you be up for another PR @malmans2 ?

@shoyer
Copy link
Member

shoyer commented Jul 17, 2021

Can we consider making typing-extensions a fully optional dependency instead?

In particular, typing-extensions 3.10 is a very new version, only from May 2021. This seems overly restrictive, particularly for a dependency that isn't really needed at all at runtime and that we only use for a few type annotations.

@shoyer shoyer mentioned this issue Jul 17, 2021
8 tasks
shoyer added a commit to shoyer/xarray that referenced this issue Jul 19, 2021
Fixes pydataGH-5495

Type checking may be a little worse if typing-extensions are not
installed, but I don't think it's worth the trouble of adding another
hard dependency just for one use for TypeGuard.
@malmans2
Copy link
Contributor Author

@shoyer I added typing-extensions in the docs too, so you'd have to remove it from there as well: https://github.com/shoyer/xarray/blob/typing-extensions-optional/doc/getting-started-guide/installing.rst

@shoyer
Copy link
Member

shoyer commented Jul 20, 2021

I made an attempt at making the use of TypeGuard optional, but it doesn't pass mypy yet: #5624

@Illviljan
Copy link
Contributor

Using typing_extensions seems to be a nice way to get backwards compatibility with older python versions.
I can see some typing bugs slipping through rather easily onsidering how picky mypy is and the CI doesn't do mypy checks on all python versions.

@max-sixty
Copy link
Collaborator

As soon as libraries like Beam allow installing later versions, I would vote to add this back.

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 a pull request may close this issue.

4 participants