-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Enable python 3.5.0-3.5.2 #2831
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
Conversation
max-sixty
commented
Mar 20, 2019
- Closes some of xarray 0.12.0 Fails To Import On Python 3.5.2 #2830
|
|
This reverts commit 77553e1.
shoyer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Max!
|
From #2830:
Unfortunately that's only supported from 3.5.2! I'll add a version check. I think there are only two places, and it won't last long |
|
I added a fix; it may have some mistakes which I'll correct Is the approach OK? It's not pretty |
xarray/coding/cftime_offsets.py
Outdated
| from .times import format_cftime_datetime | ||
|
|
||
| if USE_TYPING: | ||
| if typing.TYPE_CHECKING: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can mypy interpret this if we it this on one line? e.g., if USE_TYPING and typing.TYPE_CHECKING ?
|
Hello @max-sixty! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-03-22 02:38:08 UTC |
|
Green! Good idea re the single line, thanks @shoyer |
|
Please could someone else take a final check and merge? @shoyer reviewed but not since some changes. Thanks! |
|
This looks good to me. I tested it locally and it works with mypy. I pushed a small change to put a single |
Much nicer! |