Remove pandas.core.index.datetimelike from MyPy Blacklist #26280

makbigc · 2019-05-04T02:11:32Z

closes Typing Cleanup - Remove Blacklisted Items #25882
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2019-05-04T02:50:17Z

Codecov Report

Merging #26280 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@ Coverage Diff @@ ## master #26280 +/- ## ========================================== - Coverage 91.98% 91.98% -0.01%  ========================================== Files 175 175 Lines 52379 52379 ========================================== - Hits 48183 48179 -4  - Misses 4196 4200 +4

Flag	Coverage Δ
#multiple	`90.53% <100%> (ø)`	⬆️
#single	`40.73% <100%> (-0.15%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/datetimelike.py	`98.13% <100%> (ø)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/core/frame.py	`96.9% <0%> (-0.12%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f46ab96...1fd95eb. Read the comment docs.

codecov · 2019-05-04T02:50:17Z

Codecov Report

Merging #26280 into master will increase coverage by 0.27%.
The diff coverage is 100%.

@@ Coverage Diff @@ ## master #26280 +/- ## ========================================== + Coverage 91.71% 91.99% +0.27%  ========================================== Files 178 180 +2 Lines 50755 50774 +19 ========================================== + Hits 46552 46708 +156  + Misses 4203 4066 -137

Flag	Coverage Δ
#multiple	`90.63% <100%> (+0.32%)`	⬆️
#single	`41.85% <100%> (+0.54%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/arrays/base.py	`99.43% <100%> (ø)`	⬆️
pandas/core/indexes/datetimelike.py	`98.14% <100%> (ø)`	⬆️
pandas/core/algorithms.py	`94.74% <100%> (ø)`	⬆️
pandas/core/computation/check.py	`85.71% <0%> (-6.6%)`	⬇️
pandas/compat/__init__.py	`92% <0%> (-1.55%)`	⬇️
pandas/io/gbq.py	`88.88% <0%> (-0.59%)`	⬇️
pandas/tseries/offsets.py	`96.36% <0%> (-0.33%)`	⬇️
pandas/plotting/_matplotlib/hist.py	`76.82% <0%> (-0.19%)`	⬇️
pandas/core/frame.py	`96.89% <0%> (-0.12%)`	⬇️
pandas/plotting/_matplotlib/core.py	`88.02% <0%> (-0.09%)`	⬇️
... and 71 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5574a9f...b061ac1. Read the comment docs.

WillAyd

Thanks for the PR! Looks like there's an isort failure in CI

WillAyd · 2019-05-04T05:03:55Z

pandas/core/indexes/datetimelike.py

 _isnan = cache_readonly(DatetimeLikeArrayMixin._isnan.fget)
 hasnans = cache_readonly(DatetimeLikeArrayMixin._hasnans.fget)
 _hasnans = hasnans # for index / array -agnostic code
 _resolution = cache_readonly(DatetimeLikeArrayMixin._resolution.fget)


So it looks like these cache_readonly calls are what's tripping up mypy. @jbrockmendel might be able to offer some insight here

Is the problem with isort or mypy?

If its mypy then would it be that it wants something like # type: cache_readonly?

It's a mypy failure. Not actually with the cache_readonly as much as accessing the .fget attribute of the properties.

What's the intention of this code? Is passing fget from the property to cache_readonly faster than just accessing the property?

What's the intention of this code? Is passing fget from the property to cache_readonly faster than just accessing the property?

cache_readonly behaves like property, expecting to get a unary method as its argument. Does it even work if you don't make the .fget` explicit?

This failure is probably due to mypy. https://github.com/python/mypy/issues/6185

Deleting .fget run into other errors.

jreback · 2019-05-05T22:21:03Z

pandas/core/indexes/datetimelike.py

 return self._data._data

- @property
+ @property # type: ignore


why is this needed?

WillAyd · 2019-05-14T01:50:07Z

Can you merge master?

vaibhavhrt · 2019-05-24T23:54:59Z

@makbigc some of the errors in this file has been solved by #26404
You can go ahead and fix(or ignore) the remaining.

WillAyd · 2019-05-29T14:55:08Z

Can you merge master? Think this will be the last remaining module from the blacklist

jreback · 2019-05-30T01:31:26Z

can you merge master

makbigc · 2019-05-30T12:22:50Z

mypy has the following error for the annotated value function:

pandas/core/indexes/datetimelike.py:139: error: "None" has no attribute "_data"

I add annotation # type: DatetimeArray to _data. But mypy still has the same error.

pandas/pandas/core/indexes/datetimes.py

Line 327 in 8154efb

result._data = dtarr

Do you have any better way?

jreback · 2019-05-30T12:23:20Z

pandas/core/indexes/datetimelike.py

 # ------------------------------------------------------------------------
 # Abstract data attributes

 @property


why this change?

mypy has the following error for the annotated value function:

pandas/core/indexes/datetimelike.py:139: error: "None" has no attribute "_data"

I add annotation # type: DatetimeArray to _data. But mypy still has the same error.

pandas/pandas/core/indexes/datetimes.py

Line 327 in 8154efb

result._data = dtarr

I guess that mypy mixed up the instance attribute _data with the class attributes _data which is set None in the beginning of DatetimeIndexOpsMixin.

cc @WillAyd

Sorry missed this ping. This could be a similar conversation to https://github.com/pandas-dev/pandas/pull/26518/files#r287573810

I think a better solution is to provide the appropriate type to _data. Can you create a DatetimeLikeArray in pandas._typing which should be something like DatetimeLikeArray = TypeVar('DatetimeLikeArray', DatetimeArray, PeriodArray, TimedeltaArray) and assign that type to _data in the class? I think that should resolve.

cc @jbrockmendel in case he has other insights on the types here

WillAyd · 2019-06-03T22:50:20Z

pandas/core/indexes/datetimelike.py

 # ------------------------------------------------------------------------
 # Abstract data attributes

 @property


I think a better solution is to provide the appropriate type to _data. Can you create a DatetimeLikeArray in pandas._typing which should be something like DatetimeLikeArray = TypeVar('DatetimeLikeArray', DatetimeArray, PeriodArray, TimedeltaArray) and assign that type to _data in the class? I think that should resolve.

cc @jbrockmendel in case he has other insights on the types here

WillAyd · 2019-06-10T13:17:44Z

@makbigc can you merge master and address last comment? This should close out the reference issue (updated OP to reflect)

makbigc · 2019-06-10T15:22:12Z

I can't figure out why DatetimeArray, PeriodArray and TimedeltaArray cannot be imported inside _typing.py

from pandas.core.arrays import DatetimeArray, PeriodArray, TimedeltaArray

jreback · 2019-06-10T16:02:27Z

pandas/_typing.py

 ABCExtensionArray, ABCIndexClass, ABCSeries, ABCSparseSeries)

+from pandas.core.arrays import DatetimeArray, PeriodArray, TimedeltaArray
+


don’t try to import these use a quoted version

The following is the import error in the failed tests:
ImportError: cannot import name '_INT64_DTYPE'

The name of the module _typing.py is started with underscore. In PEP 8 (https://www.python.org/dev/peps/pep-0008/#descriptive-naming-styles ), it states

_single_leading_underscore: weak "internal use" indicator. E.g. from M import * does not import objects whose names start with an underscore.

Should we remove underscore from the name _typing? Or something else?

you can’t import these names as they would cause circular imports
and don’t need to just use ‘DatetimeArray’ and such

_INT64_DTYPE is just an alias, i.e., _INT64_DTYPE = np.dtype(np.int64). The import of numpy and the definition of _INT64_DTYPE are in the same file pandas/core/dtype/common.py.

I did a little experiment. Open a file called 'typing.py' (without underscore prefix) in the same directory as '_typing.py'. One can import DatetimeArray in typing.py.

Importing PeriodArray and TImedeltaArray also run into the same ImportError. Without them, one cannot define the mypy object TypeVar.
DatetimeLikeArray = TypeVar('DatetimeLikeArray', DatetimeArray, PeriodArray, TimedeltaArray).

@WillAyd Would you tell me your opinion? Should we change the filename of _typing.py?

WillAyd · 2019-06-18T17:34:19Z

No I don’t think this needs to be renamed. I don’t have a problem using the TYPE_CHECKING constant in that module alone if it resolves circular imports.

…

Sent from my iPhone

On Jun 18, 2019, at 10:23 AM, Mak Sze Chun ***@***.***> wrote: @makbigc commented on this pull request. In pandas/_typing.py: > @@ -11,12 +11,17 @@ from pandas.core.dtypes.generic import ( ABCExtensionArray, ABCIndexClass, ABCSeries, ABCSparseSeries) +from pandas.core.arrays import DatetimeArray, PeriodArray, TimedeltaArray + @WillAyd Would you tell me your opinion? Should we change the filename of _typing.py? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

makbigc · 2019-06-19T09:22:21Z

Yes, the problem is due to the circular import.
common -> _typing -> datetimelikey -> common
I can't resolve this circular import and datetimelike.py is the last exception in mypy.ini. So I close this PR for others picking it up.
Thanks all.

WillAyd · 2019-06-19T13:16:11Z

Doesn’t using the TYPE_CHECKING constant from the stdlib typing module in pandas._typing resolve the circular import?

I think this is really close!

makbigc · 2019-06-20T07:22:16Z

@WillAyd Thanks for your reply. I have added the following in _typing.py

if TYPE_CHECKING: from pandas.core.arrays.datetimes import DatetimeArray from pandas.core.arrays.period import PeriodArray from pandas.core.arrays.timedeltas import TimedeltaArray DatetimeLikeArray = TypeVar('DatetimeLikeArray', DatetimeArray, PeriodArray, TimedeltaArray)

and added the following to datetimelike.py

if TYPE_CHECKING: from pandas._typing import DatetimeLikeArray

The circular import is solved. But mypy cannot find the DatetimeLikeArray!
pandas/core/indexes/datetimelike.py:67: error: Invalid type "pandas.DatetimeLikeArray"

jreback · 2019-06-25T12:24:49Z

lgtm. @WillAyd merge when ready.

WillAyd · 2019-06-25T12:34:45Z

Thanks @makbigc !

WillAyd requested changes May 4, 2019

View reviewed changes

WillAyd added the Typing type annotations, mypy/pyright type checking label May 4, 2019

jreback requested changes May 5, 2019

View reviewed changes

pandas/core/indexes/datetimelike.py Outdated

return self._data._data

@property

@property # type: ignore

Copy link

Contributor

jreback May 5, 2019

Choose a reason for hiding this comment

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

why is this needed?

makbigc force-pushed the typing-GH25882 branch 2 times, most recently from 8e67d73 to 7c56581 Compare May 8, 2019 03:58

WillAyd mentioned this pull request May 15, 2019

Fix type annotations in pandas.core.indexes.datetimes #26404

Merged

4 tasks

makbigc added 3 commits May 25, 2019 21:49

Add type annotation and remove blacklist item

a3a1605

isort datetimelike.py

9c60e73

Add comment for the need of type: ignore

717bb83

makbigc force-pushed the typing-GH25882 branch from 19a6c00 to 717bb83 Compare May 25, 2019 13:55

makbigc added 2 commits May 30, 2019 18:26

merge

4b3af23

Add ignore comment

29dbd92

jreback reviewed May 30, 2019

View reviewed changes

WillAyd requested changes Jun 3, 2019

View reviewed changes

makbigc added 2 commits June 10, 2019 22:32

Add annotation DatetimeLikeArray to _data

213af4a

resolve conflict

a5f7d7b

jreback reviewed Jun 10, 2019

View reviewed changes

Import EA directly

b7f5a9c

makbigc closed this Jun 19, 2019

WillAyd reopened this Jun 24, 2019

WillAyd and others added 4 commits June 24, 2019 18:05

Removed DatetimeLikeArray TypeVar for now

2bd7542

Removed DatetimeLikeArray annotations

d9ab7b0

isort fixup

dea0cb1

Update _typing.py

b061ac1

jreback added this to the 0.25.0 milestone Jun 25, 2019

jreback approved these changes Jun 25, 2019

View reviewed changes

WillAyd merged commit 606178a into pandas-dev:master Jun 25, 2019

		ABCExtensionArray, ABCIndexClass, ABCSeries, ABCSparseSeries)

		from pandas.core.arrays import DatetimeArray, PeriodArray, TimedeltaArray

Uh oh!

Remove pandas.core.index.datetimelike from MyPy Blacklist #26280

Remove pandas.core.index.datetimelike from MyPy Blacklist #26280

Uh oh!

Conversation

makbigc commented May 4, 2019 • edited by WillAyd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

codecov bot commented May 4, 2019

Codecov Report

codecov bot commented May 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

WillAyd left a 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

Choose a reason for hiding this comment

WillAyd commented May 14, 2019

vaibhavhrt commented May 24, 2019

WillAyd commented May 29, 2019

jreback commented May 30, 2019

makbigc commented May 30, 2019

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 Jun 10, 2019

makbigc commented Jun 10, 2019

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 Jun 18, 2019 via email

makbigc commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WillAyd commented Jun 19, 2019

makbigc commented Jun 20, 2019

jreback commented Jun 25, 2019

WillAyd commented Jun 25, 2019

Labels

5 participants

makbigc commented May 4, 2019 •

edited by WillAyd

Loading

codecov bot commented May 4, 2019 •

edited

Loading

makbigc commented Jun 19, 2019 •

edited

Loading