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

TYP: annotate #32730

Merged
merged 7 commits into from
Mar 19, 2020
Merged

TYP: annotate #32730

merged 7 commits into from
Mar 19, 2020

Conversation

jbrockmendel
Copy link
Member

cc @simonjayhawkins had to revert some annotations because mypy couldn't tell that ArrayLike had "copy" or "view". Is a fix for that depending on numpy making annotations/stubs?

@@ -34,7 +34,6 @@
from pandas.core.arrays import ExtensionArray
from pandas.core.construction import create_series_with_explicit_dtype
import pandas.core.nanops as nanops
from pandas.typing import ArrayLike
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe no need to revert. this was a typo. pandas.typing -> pandas._typing

@@ -931,7 +931,7 @@ def copy(self) -> ABCExtensionArray:
"""
raise AbstractMethodError(self)

def view(self, dtype=None) -> Union[ABCExtensionArray, np.ndarray]:
def view(self, dtype=None) -> ArrayLike:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to the docstring the return type should be just ExtensionArray?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update either the doc-string or the type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you indicate when it returns an ndarray?

(I am wondering if we actually should be strict here, and tighten this method to always return an EA)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm outside of this comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you indicate when it returns an ndarray?

It isn't obvious to me how; did you have something in mind?

(I am wondering if we actually should be strict here, and tighten this method to always return an EA)

xref #24877 same idea but for astype.

@simonjayhawkins
Copy link
Member

Is a fix for that depending on numpy making annotations/stubs?

I think it's because ArrayLike is a TypeVar. without numpy stubs, numpy imports resolve to Any and shouldn't therefore cause any mypy errors.

To us a typevar as the return type for a method in a class, the typevar needs tobe bound to something. I think in this case typing.Generic is needed. The following diff on 8f7d8aa resolves the has no attribute errors

diff --git a/pandas/core/arrays/base.py b/pandas/core/arrays/base.py
index 1991c8e8c..8750265a1 100644
--- a/pandas/core/arrays/base.py
+++ b/pandas/core/arrays/base.py
@@ -931,7 +931,7 @@ class ExtensionArray:
         """
         raise AbstractMethodError(self)
 
-    def view(self, dtype=None) -> ArrayLike:
+    def view(self, dtype=None) -> "ExtensionArray":
         """
         Return a view on the array.
 
diff --git a/pandas/core/base.py b/pandas/core/base.py
index 764a6fdfd..eac7f86e8 100644
--- a/pandas/core/base.py
+++ b/pandas/core/base.py
@@ -4,11 +4,12 @@ Base and utility classes for pandas objects.
 
 import builtins
 import textwrap
-from typing import Dict, FrozenSet, List, Optional
+from typing import Dict, FrozenSet, Generic, List, Optional
 
 import numpy as np
 
 import pandas._libs.lib as lib
+from pandas._typing import ArrayLike
 from pandas.compat import PYPY
 from pandas.compat.numpy import function as nv
 from pandas.errors import AbstractMethodError
@@ -34,7 +35,6 @@ from pandas.core.algorithms import duplicated, unique1d, value_counts
 from pandas.core.arrays import ExtensionArray
 from pandas.core.construction import create_series_with_explicit_dtype
 import pandas.core.nanops as nanops
-from pandas.typing import ArrayLike
 
 _shared_docs: Dict[str, str] = dict()
 _indexops_doc_kwargs = dict(
@@ -587,7 +587,7 @@ class ShallowMixin:
         return self._constructor(obj, **kwargs)
 
 
-class IndexOpsMixin:
+class IndexOpsMixin(Generic[ArrayLike]):
     """
     Common ops mixin to support a unified interface / docs for Series / Index
     """
diff --git a/pandas/core/indexes/base.py b/pandas/core/indexes/base.py
index 319664894..48221c922 100644
--- a/pandas/core/indexes/base.py
+++ b/pandas/core/indexes/base.py
@@ -12,7 +12,7 @@ from pandas._libs.lib import is_datetime_array, no_default
 from pandas._libs.tslibs import OutOfBoundsDatetime, Timestamp
 from pandas._libs.tslibs.period import IncompatibleFrequency
 from pandas._libs.tslibs.timezones import tz_compare
-from pandas._typing import Label
+from pandas._typing import ArrayLike, Label
 from pandas.compat import set_function_name
 from pandas.compat.numpy import function as nv
 from pandas.util._decorators import Appender, Substitution, cache_readonly
@@ -181,7 +181,7 @@ def _new_Index(cls, d):
     return cls.__new__(cls, **d)
 
 
-class Index(IndexOpsMixin, PandasObject):
+class Index(IndexOpsMixin[ArrayLike], PandasObject):
     """
     Immutable ndarray implementing an ordered, sliceable set. The basic object
     storing axis labels for all pandas objects.
@@ -3840,7 +3840,7 @@ class Index(IndexOpsMixin, PandasObject):
         return array
 
     @property
-    def _values(self) -> Union[ExtensionArray, np.ndarray]:
+    def _values(self) -> ArrayLike:
         """
         The best array representation.
 

@simonjayhawkins
Copy link
Member

simonjayhawkins commented Mar 15, 2020

as a follow-on could then do either class ExtensionIndex(Index[ExtensionArray]): which would make the subclass of the generic class non-generic or class ExtensionIndex(Index[ExtensionArrayT]): and define ExtensionArrayT as a typevar bound only by ExtensionArray and keep ExtensionIndex as generic and the make the subclasses non-geneirc. i.e class CategoricalIndex(ExtensionIndex[Categorical], accessor.PandasDelegate):

updated: typo class CategoricalIndex(Categorical[], accessor.PandasDelegate): -> class CategoricalIndex(ExtensionIndex[Categorical], accessor.PandasDelegate):

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jbrockmendel lgtm. some comments/suggestions (for follow-up), not blockers.

pandas/io/pytables.py Outdated Show resolved Hide resolved
pandas/core/arrays/period.py Show resolved Hide resolved
@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Mar 15, 2020
@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Mar 15, 2020
@jbrockmendel
Copy link
Member Author

are any of the remaining discussion points actionable?

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@simonjayhawkins simonjayhawkins merged commit dbd7a5d into pandas-dev:master Mar 19, 2020
@simonjayhawkins
Copy link
Member

Thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the annotate branch March 19, 2020 15:43
SeeminSyed pushed a commit to CSCD01-team01/pandas that referenced this pull request Mar 22, 2020
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Mar 23, 2020
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants