Fix type annotations in pandas.core.indexes.period #26518

vaibhavhrt · 2019-05-25T00:10:42Z

closes Fix type annotations in pandas.core.indexes.period #26517
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff

codecov · 2019-05-25T00:49:35Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #26518      +/-   ##
==========================================
- Coverage   91.76%   91.75%   -0.01%     
==========================================
  Files         174      174              
  Lines       50673    50673              
==========================================
- Hits        46498    46493       -5     
- Misses       4175     4180       +5

Flag	Coverage Δ
#multiple	`90.26% <100%> (ø)`	⬆️
#single	`41.7% <100%> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/period.py	`92.58% <100%> (ø)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/core/frame.py	`97% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.6% <0%> (-0.11%)`	⬇️

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 374478c...b311b66. Read the comment docs.

codecov · 2019-05-25T00:49:39Z

Codecov Report

Merging #26518 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26518      +/-   ##
==========================================
+ Coverage   91.76%   91.76%   +<.01%     
==========================================
  Files         174      174              
  Lines       50673    50649      -24     
==========================================
- Hits        46498    46478      -20     
+ Misses       4175     4171       -4

Flag	Coverage Δ
#multiple	`90.3% <100%> (+0.04%)`	⬆️
#single	`41.67% <100%> (-0.12%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/period.py	`92.58% <100%> (ø)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/io/excel/_base.py	`91.81% <0%> (-0.33%)`	⬇️
pandas/core/frame.py	`97% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.6% <0%> (-0.11%)`	⬇️
pandas/core/config_init.py	`96.85% <0%> (-0.1%)`	⬇️
pandas/core/sparse/scipy_sparse.py	`100% <0%> (ø)`	⬆️
pandas/core/series.py	`93.61% <0%> (ø)`	⬆️
pandas/core/sparse/frame.py	`95.65% <0%> (+0.01%)`	⬆️
pandas/core/sparse/series.py	`93.24% <0%> (+0.06%)`	⬆️
... and 5 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 374478c...3e1a81a. Read the comment docs.

gfyoung

Well, that was easy. Nice! 👍

@WillAyd : If you want to have a look before merging

vaibhavhrt · 2019-05-25T13:04:26Z

@gfyoung yeah, I wanted to do it in #26404 but it got merged before I can make changes.

WillAyd · 2019-05-25T20:08:51Z

pandas/core/indexes/period.py

@@ -168,7 +168,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin):
    _is_numeric_dtype = False
    _infer_as_myclass = True

-    _data = None  # type: PeriodArray
+    _data = None


So what is this getting inferred as now?

@WillAyd Inferred type is None, which is not correct, but putting the actual type is almost impossible. Because the type of _data in parent/sibling classes (e.g. in class Index, DatetimeIndexOpsMixin, etc.) is different. If we want to properly type this we need to create a type ArrayLike(I guess) in pandas._typing which can contain all possible type for _data then use it everywhere. But I don't know how many of these arraylike types(PeriodArray, DatetimeLikeArray, etc) are there, so someone will have to tell me how to find that, maybe @gfyoung can help with that.
If you want something like this, I would suggest doing that in a separate issue & PR as a lot of files will be changed as _data is in like 100 different files.

We already have that in the typing module - did that not work?

Okay, we do have array like in _typing, my bad, didn't saw that.
I think _typing.AnyArrayLike is what we need. Mypy will be happy with whatever type I put for _data, so someone should confirm that AnyArrayLike is the proper type to use for _data, and I will type _data in every class.

Yea I think that should work here

WillAyd

@vaibhavhrt what is the purpose of this PR? I don't see any mypy failures in CI nor locally on master

WillAyd · 2019-05-26T06:36:12Z

pandas/core/indexes/period.py

@@ -168,7 +168,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin):
    _is_numeric_dtype = False
    _infer_as_myclass = True

-    _data = None  # type: PeriodArray
+    _data = None


We already have that in the typing module - did that not work?

jreback · 2019-05-26T14:33:53Z

pandas/core/indexes/period.py

@@ -80,7 +80,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin):

    Parameters
    ----------
-    data : array-like (1-dimensional), optional
+    data : array-like (1-dimensional) - PeriodArray, optional


this is not very clear, can you enumerate a bit

1d int np.ndarray or PeriodArray, optional

data : array-like (1-dimensional integer np.ndarray or PeriodArray), optional, looks good?

Okay, I will get it done after a few hours

@jreback I have fixed it, sorry for the delay.

vaibhavhrt · 2019-05-26T16:23:38Z

@WillAyd mypy should give this error:

pandas\core\indexes\period.py:171: error: Incompatible types in assignment (expression has type "PeriodArray", base class "DatetimeIndexOpsMixin" defined the type as "None")

Can you double check.

WillAyd · 2019-05-27T00:00:50Z

I am not getting that error locally nor is it showing in the build. What version of mypy are you using?

vaibhavhrt · 2019-05-27T00:56:55Z

Hmm, let me double check.

…

On Mon, 27 May 2019, 5:31 am William Ayd, ***@***.***> wrote: I am not getting that error locally nor is it showing in the build. What version of mypy are you using? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26518?email_source=notifications&email_token=AFMJMLFFPBOUUX7NRDK4PVTPXMQFTA5CNFSM4HPTL2JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWIP6BI#issuecomment-496041733>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFMJMLESB65Q2CBRERGMP53PXMQFTANCNFSM4HPTL2JA> .

WillAyd · 2019-05-27T01:04:55Z

Might be a misunderstanding on my end I didn’t modify mypy.ini before running. Thought this was a continuation of a prior PR but if not that’s probably it. Can check later

…

Sent from my iPhone

On May 26, 2019, at 9:34 AM, Vaibhav Vishal ***@***.***> wrote: @vaibhavhrt commented on this pull request. In pandas/core/indexes/period.py: > @@ -80,7 +80,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin): Parameters ---------- - data : array-like (1-dimensional), optional + data : array-like (1-dimensional) - PeriodArray, optional data : array-like (1-dimensional integer np.ndarray or PeriodArray), optional, looks good? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

vaibhavhrt · 2019-05-27T01:17:39Z

`pandas.core.indexes.period` needs to be removed from mypy.ini. I wanted to fix this error in a previous PR because `pandas.core.indexes.period` had a lot of other errors too relating to incompatible types which got automatically solved while trying to fix types in some other file. Just this one remained so a new PR was not absolutely necessary, but since that PR(#26404 I think) got merged before I could fix `pandas.core.indexes.period` too, so a new PR, sorry for the confusion. On Mon, May 27, 2019 at 6:35 AM William Ayd <[email protected]> wrote:

…

Might be a misunderstanding on my end I didn’t modify mypy.ini before running. Thought this was a continuation of a prior PR but if not that’s probably it. Can check later Sent from my iPhone > On May 26, 2019, at 9:34 AM, Vaibhav Vishal ***@***.***> wrote: > > @vaibhavhrt commented on this pull request. > > In pandas/core/indexes/period.py: > > > @@ -80,7 +80,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin): > > Parameters > ---------- > - data : array-like (1-dimensional), optional > + data : array-like (1-dimensional) - PeriodArray, optional > data : array-like (1-dimensional integer np.ndarray or PeriodArray), optional, looks good? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26518?email_source=notifications&email_token=AFMJMLCZ6RZW7XVCR3NESCLPXMXWLA5CNFSM4HPTL2JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWIRHXA#issuecomment-496047068>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFMJMLAWOYH5N2IC5JMTNZTPXMXWLANCNFSM4HPTL2JA> .

WillAyd

@vaibhavhrt all good was just a misunderstanding on my end

WillAyd · 2019-05-27T02:48:19Z

pandas/core/indexes/period.py

@@ -168,7 +168,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin):
    _is_numeric_dtype = False
    _infer_as_myclass = True

-    _data = None  # type: PeriodArray
+    _data = None


Yea I think that should work here

jreback · 2019-05-27T14:49:32Z

pandas/core/indexes/period.py

@@ -80,7 +80,7 @@ class PeriodIndex(DatetimeIndexOpsMixin, Int64Index, PeriodDelegateMixin):

    Parameters
    ----------
-    data : array-like (1-dimensional), optional
+    data : array-like (1-dimensional) - PeriodArray, optional


jreback · 2019-05-29T12:40:44Z

thanks @vaibhavhrt

vaibhavhrt added 2 commits May 25, 2019 05:30

remove the module from mypy.ini

92e9769

remove type comment

b311b66

vaibhavhrt mentioned this pull request May 25, 2019

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

Merged

4 tasks

gfyoung added Indexing Related to indexing on series/frames, not to indexes themselves Typing type annotations, mypy/pyright type checking labels May 25, 2019

gfyoung approved these changes May 25, 2019

View reviewed changes

WillAyd reviewed May 25, 2019

View reviewed changes

WillAyd added this to the 0.25.0 milestone May 25, 2019

WillAyd requested changes May 26, 2019

View reviewed changes

jreback requested changes May 26, 2019

View reviewed changes

WillAyd reviewed May 27, 2019

View reviewed changes

jreback requested changes May 27, 2019

View reviewed changes

update docstring

3e1a81a

jreback merged commit a91da0c into pandas-dev:master May 29, 2019

vaibhavhrt deleted the pandas.core.indexes.period branch May 29, 2019 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix type annotations in pandas.core.indexes.period #26518

Fix type annotations in pandas.core.indexes.period #26518

vaibhavhrt commented May 25, 2019 •

edited

Loading

codecov bot commented May 25, 2019

codecov bot commented May 25, 2019 •

edited

Loading

gfyoung left a comment

vaibhavhrt commented May 25, 2019

WillAyd May 25, 2019

vaibhavhrt May 26, 2019

WillAyd May 26, 2019

vaibhavhrt May 26, 2019

WillAyd May 27, 2019

WillAyd left a comment

WillAyd May 26, 2019

jreback May 26, 2019

vaibhavhrt May 26, 2019

jreback May 27, 2019

vaibhavhrt May 27, 2019

vaibhavhrt May 29, 2019

vaibhavhrt commented May 26, 2019

WillAyd commented May 27, 2019

vaibhavhrt commented May 27, 2019 via email

WillAyd commented May 27, 2019 via email

vaibhavhrt commented May 27, 2019 via email

WillAyd left a comment

WillAyd May 27, 2019

jreback May 27, 2019

jreback commented May 29, 2019

Fix type annotations in pandas.core.indexes.period #26518

Fix type annotations in pandas.core.indexes.period #26518

Conversation

vaibhavhrt commented May 25, 2019 • edited Loading

codecov bot commented May 25, 2019

Codecov Report

codecov bot commented May 25, 2019 • edited Loading

Codecov Report

gfyoung left a comment

Choose a reason for hiding this comment

vaibhavhrt commented May 25, 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 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

vaibhavhrt commented May 26, 2019

WillAyd commented May 27, 2019

vaibhavhrt commented May 27, 2019 via email

WillAyd commented May 27, 2019 via email

vaibhavhrt commented May 27, 2019 via email

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

jreback commented May 29, 2019

vaibhavhrt commented May 25, 2019 •

edited

Loading

codecov bot commented May 25, 2019 •

edited

Loading