CLN: Fixing mypy errors in pandas/conftest.py #29046

angelaambroz · 2019-10-17T00:30:25Z

xref Fix mypy errors for pandas\tests\* #28926
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
~~whatsnew entry~~

WillAyd · 2019-10-17T00:46:45Z

Can you annotate these instead of ignoring? I think should can just use Dtype from pandas._typing. Might need to modify that to account for built in types

…

Sent from my iPhone

On Oct 16, 2019, at 5:30 PM, Angela Ambroz ***@***.***> wrote: addresses, but does not close, #28926 tests added / passed passes black pandas passes git diff upstream/master -u -- "*.py" | flake8 --diff whatsnew entry You can view, comment on, or merge this pull request online at: #29046 Commit Summary Ignoring some mypy errors related to imports and list inference File Changes M pandas/conftest.py (10) Patch Links: https://github.com/pandas-dev/pandas/pull/29046.patch https://github.com/pandas-dev/pandas/pull/29046.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

angelaambroz · 2019-10-17T00:52:30Z

Cool - hadn't seen pandas._typing. Will do.

pep8speaks · 2019-10-17T01:26:47Z

Hello @angelaambroz! 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 2020-03-25 18:36:23 UTC

angelaambroz · 2019-10-17T01:30:13Z

I've left the # type: ignore for the import statement, since I suspect it's related to what the mypy docs describe when a module is present, but mypy is unable - for whatever reason - to check its type hints.

WillAyd · 2019-10-17T01:36:34Z

pandas/conftest.py

@@ -481,7 +482,7 @@ def tz_aware_fixture(request):

 UNSIGNED_INT_DTYPES = ["uint8", "uint16", "uint32", "uint64"]
 UNSIGNED_EA_INT_DTYPES = ["UInt8", "UInt16", "UInt32", "UInt64"]
-SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"]
+SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"]  # type: Dtype


This is actually a List[Dtype]

I think Dtype needs to be updated in pandas._typing to include Type[Union[float, int, str, bool]] for when the built-in values get provided - could you make that change as well?

Yep, sure - thanks for the guidance.

Type[Union[float, str, bool]] would be sufficient as int is duck type compatible with float see https://mypy.readthedocs.io/en/latest/duck_type_compatibility.html#duck-type-compatibility

Thanks, both. Just pushed what I think you both meant - let me know. I'm still learning about types! Looks like complex was also duck typed to float.

WillAyd · 2019-10-17T01:37:05Z

pandas/conftest.py

@@ -8,9 +8,10 @@
 from hypothesis import strategies as st
 import numpy as np
 import pytest
-from pytz import FixedOffset, utc
+from pytz import FixedOffset, utc  # type: ignore


This is the same as #28932 - I think this needs to be fixed upstream in Typeshed. Would you be interested in submitting that there?

Sure - I can give it a shot.

Picking this up again - just got the changes merged in typeshed (python/typeshed#3393). I see that there are now some conflicts with master; I can address those and pick this up again in the next couple days.

Great job and thanks for fixing that upstream! Still not a rush on our end but should make it into the next mypy release. Will be helpful for us in a few places

WillAyd · 2019-10-19T00:19:39Z

You should encapsulate those in Type[...] otherwise this would be looking for an instance of those. FWIW I’d rather be explicit about these rather than relying on any hierarchy or inference. In the future we’d probably encapsulate in Literal

…

Sent from my iPhone

On Oct 18, 2019, at 4:50 PM, Angela Ambroz ***@***.***> wrote: @angelaambroz commented on this pull request. In pandas/conftest.py: > @@ -481,7 +482,7 @@ def tz_aware_fixture(request): UNSIGNED_INT_DTYPES = ["uint8", "uint16", "uint32", "uint64"] UNSIGNED_EA_INT_DTYPES = ["UInt8", "UInt16", "UInt32", "UInt64"] -SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"] +SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"] # type: Dtype Thanks, both. Just pushed what I think you both meant - let me know. I'm still learning about types! Looks like complex was also duck typed to float. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

pandas/_typing.py

WillAyd · 2019-10-21T21:36:59Z

pandas/_typing.py

@@ -18,7 +18,7 @@
 AnyArrayLike = TypeVar("AnyArrayLike", "ExtensionArray", "Index", "Series", np.ndarray)
 ArrayLike = TypeVar("ArrayLike", "ExtensionArray", np.ndarray)
 DatetimeLikeScalar = TypeVar("DatetimeLikeScalar", "Period", "Timestamp", "Timedelta")
-Dtype = Union[str, np.dtype, "ExtensionDtype"]
+Dtype = Type[Union[str, float, bool, np.dtype, "ExtensionDtype"]]


Suggested change

Dtype = Type[Union[str, float, bool, np.dtype, "ExtensionDtype"]]

Dtype = Union[ExtensionDtype, Type[Union[str, float, int, complex, bool]]]

Somewhat more complicated but the ExtensionDtype needs to be an instance, so should exclude from the Type element.

To illustrate:

>>> pd.Series(range(1), dtype=pd.Int64Dtype) # Not what you would expect 0 0 dtype: object >>> pd.Series(range(1), dtype=pd.Int64Dtype()) # This instance works though 0 0 dtype: object >>> pd.Series(range(1), dtype=int) # contrast to the builtin, which uses Type[int] 0 0 dtype: int64

cc @TomAugspurger in case there's something I'm missing with the ExtensionDtype

Also @simonjayhawkins do you mind if we include int in the Union? Understood that it gets replaced as part of the mypy type hierarchy, but given dtype=int is a valid construct it serves as literal documentation and requires less knowledge of how mypy works. Whenever we get to Python 3.8 as the min version these should be converted to Literal annotations anyway

>>> pd.Series(range(1), dtype=pd.Int64Dtype)

should probably raise. I don't think we can automatically make that work since we have parametrized dtypes. Not sure what that does to the type hint.

Also @simonjayhawkins do you mind if we include int in the Union?

no problem.

👋 Sorry for the silence on my end. I'm picking this up again now. I didn't follow your comment, @WillAyd (though I committed your suggestion - hence the Outdated flag). Does this requested review still apply? Thanks.

alimcmaster1 · 2019-11-17T16:23:16Z

@angelaambroz thanks for the PR! Are you still working on this?

angelaambroz · 2019-11-17T19:22:57Z

Eek, sorry, yes. I'd like to keep working on this. Apologies for the silence.

simonjayhawkins · 2019-11-21T16:32:43Z

pandas/conftest.py

@@ -481,14 +483,14 @@ def tz_aware_fixture(request):

 UNSIGNED_INT_DTYPES = ["uint8", "uint16", "uint32", "uint64"]
 UNSIGNED_EA_INT_DTYPES = ["UInt8", "UInt16", "UInt32", "UInt64"]
-SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"]
+SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"]  # type: List[Dtype]


will need to change to py3.6 syntax.

simonjayhawkins · 2019-11-21T16:32:50Z

pandas/conftest.py

-STRING_DTYPES = [str, "str", "U"]
+FLOAT_DTYPES = [float, "float32", "float64"]  # type: List[Dtype]
+COMPLEX_DTYPES = [complex, "complex64", "complex128"]  # type: List[Dtype]
+STRING_DTYPES = [str, "str", "U"]  # type: List[Dtype]


angelaambroz · 2019-12-13T21:00:55Z

CI fails on pytest pandas/conftest.py, due to ExtensionDtype not being recognized. It's currently only imported if TYPE_CHECKING==True. There's a comment warning against circular imports, so I'm hesitating moving

from pandas.core.dtypes.dtypes import ExtensionDtype  # noqa: F401

out of the conditional. Halp? 🆘

pandas/_typing.py

datapythonista · 2019-12-29T14:03:40Z

@angelaambroz can you take care of the last comment, so we can finally merge this please? Please ping us once the CI is green, for a faster merge.

angelaambroz · 2019-12-29T14:07:09Z

Argh, sorry, I'm traveling and away from a laptop until Jan 8. Can someone else do the last push? Sorry it's been hanging so long.

…

On Sun, Dec 29, 2019, 3:03 PM Marc Garcia ***@***.***> wrote: @angelaambroz <https://github.com/angelaambroz> can you take care of the last comment, so we can finally merge this please? Please ping us once the CI is green, for a faster merge. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#29046?email_source=notifications&email_token=AAWJPII4O7ZCSMV5MV2CMX3Q3CU35A5CNFSM4JBSTLY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZAFOA#issuecomment-569508536>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWJPIJPHDLH5ZZRYN4MX7DQ3CU35ANCNFSM4JBSTLYQ> .

datapythonista · 2019-12-29T14:27:32Z

You can do the last commit from the website, there is a dropdown to accept the proposed change. I tried to do it myself, but looks like GitHub just let the author accept it.

Co-Authored-By: William Ayd <william.ayd@icloud.com>

angelaambroz · 2019-12-29T14:48:29Z

Ack. CI failing linting - it doesn't like the import order in conftest.

I could try fixing this via the GitHub web UI (iffy, but I might be able to do it).
Someone else picks this up.
We wait until Jan 8/9, when I'm back at a computer and can debug properly.

alimcmaster1 · 2020-01-01T13:55:37Z

Ack. CI failing linting - it doesn't like the import order in conftest.

I could try fixing this via the GitHub web UI (iffy, but I might be able to do it).

Someone else picks this up.

We wait until Jan 8/9, when I'm back at a computer and can debug properly.

You can run isort -rc pandas to fix the import order issue. For more info see here: https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#import-formatting

WillAyd · 2020-01-16T20:36:43Z

@angelaambroz can you fix up merge conflict? I think this is pretty much ready

angelaambroz · 2020-01-17T14:24:51Z

It's failing on type checks now; this was unexpected, but I didn't dive deep into the merge conflicts on pandas/_typing.py, where I saw some changes (I intended just to change the Dtype type). I'll try to go through this later today and see what's up.

WillAyd

I think just need to add str as a valid dtype to resolve CI errors. Something like dtype="boolean" is valid

WillAyd · 2020-01-17T23:39:40Z

pandas/_typing.py

@@ -44,7 +45,7 @@

 # other

-Dtype = Union[str, np.dtype, "ExtensionDtype"]
+Dtype = Union["ExtensionDtype", Type[Union[str, float, int, complex, bool]]]


Suggested change

Dtype = Union["ExtensionDtype", Type[Union[str, float, int, complex, bool]]]

Dtype = Union["ExtensionDtype", str, Type[Union[str, float, int, complex, bool]]]

simonjayhawkins · 2020-02-09T18:15:02Z

@angelaambroz can you merge master

MarcoGorelli · 2020-02-10T13:06:56Z

Hi @angelaambroz - sorry to chase you up, just wanted to ask whether you're still working on this :)

angelaambroz · 2020-02-10T15:00:09Z

@MarcoGorelli Feel free to grab it - I'm being very slow and not finding enough time to get back to this! I'd be happy to continue, but don't want to hold things up. Thanks for checking in!

MarcoGorelli · 2020-02-11T10:16:25Z

I'd be happy to continue, but don't want to hold things up

It's OK, I don't believe this is too urgent, so feel free to continue and to ask for help if you're stuck

WillAyd · 2020-03-11T03:15:02Z

@angelaambroz any interest in fixing up the merge conflict and addressing last comment?

angelaambroz · 2020-03-11T16:58:56Z

👋 Yes, here! I hang my head in shame. But I can do this later today.

WillAyd · 2020-03-25T23:51:19Z

Thanks @angelaambroz for seeing this through

Ignoring some mypy errors related to imports and list inference

bfc175e

angelaambroz changed the title ~~Ignoring some mypy errors related to imports and list inference~~ CLN: Fixing mypy errors in pandas/conftest.py Oct 17, 2019

Adding Dtype

6408388

Two spaces

f4afbb2

WillAyd requested changes Oct 17, 2019

View reviewed changes

simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Oct 17, 2019

simonjayhawkins mentioned this pull request Oct 17, 2019

Fix mypy errors for pandas\tests\* #28926

Closed

38 tasks

WillAyd mentioned this pull request Oct 18, 2019

Remove numpy references from pandas._typing #29064

Closed

angelaambroz added 2 commits October 18, 2019 19:47

MR comments

ce2d9b7

MR comments, removing type ignore

96a09ec

Type Union etc

f07b7a0

simonjayhawkins reviewed Oct 21, 2019

View reviewed changes

pandas/_typing.py Outdated Show resolved Hide resolved

Forgot Type

77543f2

WillAyd requested changes Oct 21, 2019

View reviewed changes

angelaambroz added 2 commits November 2, 2019 19:12

PR comments

50dadb2

Merge conflict importing Type in _typing

5bce559

simonjayhawkins reviewed Nov 21, 2019

View reviewed changes

angelaambroz added 2 commits December 13, 2019 15:21

Merge remote-tracking branch 'upstream/master' into issue28926_conftest

1856a58

PR comments

4be05dc

WillAyd requested changes Dec 13, 2019

View reviewed changes

pandas/_typing.py Outdated Show resolved Hide resolved

Update pandas/_typing.py

3fbf980

Co-Authored-By: William Ayd <william.ayd@icloud.com>

simonjayhawkins mentioned this pull request Dec 31, 2019

TYP: Annotations in pandas/core/nanops.py #30461

Merged

5 tasks

angelaambroz added 2 commits January 16, 2020 18:36

Merge conflicts

05bb32b

Fixing import sort order

4b8b565

WillAyd requested changes Jan 17, 2020

View reviewed changes

angelaambroz added 2 commits March 25, 2020 14:08

Merge conflicts

a920f28

Linting

dd279e9

WillAyd approved these changes Mar 25, 2020

View reviewed changes

WillAyd added this to the 1.1 milestone Mar 25, 2020

WillAyd merged commit 9616d98 into pandas-dev:master Mar 25, 2020

	Dtype = Type[Union[str, float, bool, np.dtype, "ExtensionDtype"]]
	Dtype = Union[ExtensionDtype, Type[Union[str, float, int, complex, bool]]]

	Dtype = Union["ExtensionDtype", Type[Union[str, float, int, complex, bool]]]
	Dtype = Union["ExtensionDtype", str, Type[Union[str, float, int, complex, bool]]]

CLN: Fixing mypy errors in pandas/conftest.py #29046

CLN: Fixing mypy errors in pandas/conftest.py #29046

Conversation

angelaambroz commented Oct 17, 2019 • edited by simonjayhawkins Loading

WillAyd commented Oct 17, 2019 via email

angelaambroz commented Oct 17, 2019

pep8speaks commented Oct 17, 2019 • edited Loading

Comment last updated at 2020-03-25 18:36:23 UTC

angelaambroz commented Oct 17, 2019

Choose a reason for hiding this comment

WillAyd Oct 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Oct 19, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alimcmaster1 commented Nov 17, 2019

angelaambroz commented Nov 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angelaambroz commented Dec 13, 2019

datapythonista commented Dec 29, 2019

angelaambroz commented Dec 29, 2019 via email

datapythonista commented Dec 29, 2019

angelaambroz commented Dec 29, 2019 • edited Loading

alimcmaster1 commented Jan 1, 2020

WillAyd commented Jan 16, 2020

angelaambroz commented Jan 17, 2020

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins commented Feb 9, 2020

MarcoGorelli commented Feb 10, 2020

angelaambroz commented Feb 10, 2020

MarcoGorelli commented Feb 11, 2020

WillAyd commented Mar 11, 2020

angelaambroz commented Mar 11, 2020

WillAyd commented Mar 25, 2020

angelaambroz commented Oct 17, 2019 •

edited by simonjayhawkins

Loading

pep8speaks commented Oct 17, 2019 •

edited

Loading

WillAyd Oct 17, 2019 •

edited

Loading

angelaambroz commented Dec 29, 2019 •

edited

Loading