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

Remove numpy references from pandas._typing #29064

Closed
WillAyd opened this issue Oct 18, 2019 · 7 comments
Closed

Remove numpy references from pandas._typing #29064

WillAyd opened this issue Oct 18, 2019 · 7 comments
Labels
Typing type annotations, mypy/pyright type checking

Comments

@WillAyd
Copy link
Member

WillAyd commented Oct 18, 2019

Because we don't have type information from numpy AND because we specifically ignore_missing_imports for that in our setup, any numpy types currently resolve to Any. This may cause surprising behavior if not scrutinized...

To illustrate, we currently define DType in pandas._typing as Dtype = Union[str, np.dtype, "ExtensionDtype"]. The numpy item here basically makes this a Union[..., Any] so it will almost never fail static analysis. If you remove the np reference therein you will get errors like the following:

pandas/tests/arrays/sparse/test_dtype.py:74: error: Argument 1 to "SparseDtype" has incompatible type "Type[int]"; expected "Union[str, ExtensionDtype]"

In this particular case we probably want to replace np.dtype with Type[Union[float, bool, int, str, object]] to account for legitimate dtype=object calls (@angelaambroz this was mentioned in #29046).

The appearances in TypeVar items may not be as big of a deal, so open to thoughts on whomever wants to review this

@simonjayhawkins

@WillAyd WillAyd added the Typing type annotations, mypy/pyright type checking label Oct 18, 2019
@jbrockmendel
Copy link
Member

@simonjayhawkins
Copy link
Member

xref #27589

#28831 (comment) suggests that we currently could have around 62 errors to resolve if numpy stubs are added.

@WillAyd
Copy link
Member Author

WillAyd commented Oct 18, 2019

There is an open issue for numpy stubs already #27589 if we want to discuss adding that there, but I still think for now easy to remove these from pandas._typing and wait until they are actually useful to put in

@simonjayhawkins
Copy link
Member

personally, I just avoid adding types that I know will resolve to Any, so tend not to use the types in _typing, but see no reason to remove them.

o illustrate, we currently define DType in pandas._typing as Dtype = Union[str, np.dtype, "ExtensionDtype"]. The numpy item here basically makes this a Union[..., Any] so it will almost never fail static analysis.

I don't think this is entirely true. mypy can still check against the known types. so in this case only string methods and methods in ExtensionDtype can be called, otherwise mypy will report an error.

from typing import Any, Union


def foo(a: Union[str, Any]):
    a.bar
$ mypy test.py
test.py:5: error: Item "str" of "Union[str, Any]" has no attribute "bar"
Found 1 error in 1 file (checked 1 source file)

@WillAyd
Copy link
Member Author

WillAyd commented Oct 18, 2019

Hmm that's fair. In any case can still lead to very surprising behavior though like this:

https://github.com/pandas-dev/pandas/pull/29046/files#r335777289

which is what brought this to mind

@simonjayhawkins
Copy link
Member

@WillAyd With numpy types imminent, I think we can now close this issue. feel free to reopen if disagree.

@WillAyd
Copy link
Member Author

WillAyd commented Jan 12, 2021 via email

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

No branches or pull requests

3 participants