Update type hints to python3 style (non-array files) #25802
Conversation
|
I'm unsure about the method I used to define TypeVars, so an instance method could refer to its own class type, e.g., in 014c48e Is there a more appropriate way or a better place for the definition? |
|
@gwrome thanks! I don't think we need any TypeVars just yet - can you not just implicitly break lines within brackets for container types as required? |
Codecov Report
@@ Coverage Diff @@
## master #25802 +/- ##
==========================================
+ Coverage 91.27% 91.27% +<.01%
==========================================
Files 173 173
Lines 53002 53014 +12
==========================================
+ Hits 48375 48387 +12
Misses 4627 4627
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #25802 +/- ##
==========================================
- Coverage 91.48% 91.47% -0.01%
==========================================
Files 175 175
Lines 52885 52872 -13
==========================================
- Hits 48380 48366 -14
- Misses 4505 4506 +1
Continue to review full report at Codecov.
|
|
@WillAyd TIL about forward references in type hints, so that fixed what I was trying to address with TypeVars. They're all gone now. |
pandas/core/indexes/base.py
Outdated
| @@ -3632,8 +3632,7 @@ def values(self): | |||
| return self._data.view(np.ndarray) | |||
|
|
|||
| @property | |||
| def _values(self): | |||
| # type: () -> Union[ExtensionArray, Index, np.ndarray] | |||
| def _values(self) -> Union[ExtensionArray, 'Index', np.ndarray]: | |||
There was a problem hiding this comment.
Is Index not a valid type here?
There was a problem hiding this comment.
Because it's used inside the Index class itself, Index isn't defined yet when types are checked, so we need to use a forward reference literal. See PEP 484. In that context, Index alone is an unresolved reference.
|
Any guess as to why the MacOS 3.5 build fails? Did something change with respect to type hints between 3.5 and 3.6? |
|
Can you run the test suite locally? The builds are pointing to a syntax error |
|
Ah never mind I think the problem is that type annotations for class and instance variables came along later as part of PEP 526 in Python36, so for those probably best to stick with comment until we drop Py35 support |
|
I returned the two variables class/instance variables to the previous style and left the rest of the changes intact. |
|
we need a minimal run of mypy to validate things. it can be in this PR or a pre-cursor. |
|
@jreback do you want that set up as part of CI or just validate output locally? I think the former is going to take a while as a pre-cursor so I figured switch over syntax first then clean up types, though open to however you want to approach |
The bad news is that running mypy on the changed files throws a ton of errors. The less bad news is that it appears that most of the errors are outside the scope of the changes in this PR. I have some idea of how to proceed with the issues created by these type hints. The broader problems, though, will probably take substantial work outside the scope of this PR. All the commands below were run using these numpy stubs. pandas/core/base.pyI think these type hints are fine, but there are other errors, including an issue related to numpy, a missing pandas stub (which I presume hasn't been written yet), and some attribute errors in this file.
pandas/core/common.pyThe hints seem fine, but there are other errors, including an issue related to numpy, and a missing attribute error.
The remaining errors are similar to the other files. pandas/core/dtypes/base.pyThis one has a hint-related error: Importing Type from typing and replacing It also has similar numpy and missing attribute errors.
pandas/core/frame.pyMiscellaneous errors unrelated to these hints.
pandas/core/groupby/groupby.pyThis one has a few relevant errors: Almost all are a mismatch between Line 1749's error can be corrected by adding
pandas/core/indexes/base.pyMiscellaneous errors unrelated to these hints.
pandas/core/indexes/datetimelike.pyMiscellaneous errors unrelated to these hints. ``` $ MYPYPATH=stubs/ mypy --follow-imports skip ./pandas/core/indexes/datetimelike.py stubs/numpy/__init__.pyi:48: error: Class numpy.flatiter has abstract attributes "__next__" stubs/numpy/__init__.pyi:48: note: If it is meant to be abstract, add 'abc.ABCMeta' as an explicit metaclass pandas/core/indexes/datetimelike.py:136: error: Decorated property not supported pandas/core/indexes/datetimelike.py:206: error: Name '_box_values' already defined on line 71 pandas/core/indexes/datetimelike.py:701: error: Need type annotation for '_raw_methods' pandas/core/indexes/datetimelike.py:703: error: Need type annotation for '_raw_properties' ``` pandas/core/internals/blocks.pyMiscellaneous errors unrelated to these hints.
pandas/core/internals/managers.pyNumpy errors.
|
we absolutely need CI for this, which excludes non-fixed files. |
|
@gwrome can you merge master? |
|
You got it, @WillAyd |
|
thanks @gwrome |
git diff upstream/master -u -- "*.py" | flake8 --diffFixes Update Typing Comments to Python3 Syntax #25790 (non-array part)