Skip to content

Conversation

@vaibhavhrt
Copy link
Contributor

@vaibhavhrt vaibhavhrt commented Mar 28, 2019

Also remove this module from mypy.ini

@WillAyd WillAyd added the Typing type annotations, mypy/pyright type checking label Mar 28, 2019
@WillAyd
Copy link
Member

WillAyd commented Mar 28, 2019

@vaibhavhrt could you also take care of the following?

pandas/core/frame.py:368: error: Need type annotation for '_accessors'

Very similar problem mentioned in issue I think makes sense to knock out together

@vaibhavhrt
Copy link
Contributor Author

@WillAyd Sure I will take care of frames.py too

@vaibhavhrt
Copy link
Contributor Author

vaibhavhrt commented Mar 28, 2019

There is a little problem with frame.py, I am getting this error instead of what you wrote in your comment:

pandas\core\frame.py:368: error: Incompatible types in assignment (expression has type "Set[<nothing>]", base class "NDFrame" defined the type as "FrozenSet[str]")

And I can't fix it even after adding type. Still getting the same(almost) error.

pandas\core\frame.py:368: error: Incompatible types in assignment (expression has type "Set[str]", base class "NDFrame" defined the type as "FrozenSet[str]")

Most likely because I just set type of _accessor to FrozenSet in generic.py and the class DataFrame extends the class I just edited in generic.py. I think _accessor of both class needs to be same type. So which type should I use, set or frozenset

@WillAyd
Copy link
Member

WillAyd commented Mar 28, 2019 via email

@vaibhavhrt
Copy link
Contributor Author

vaibhavhrt commented Mar 28, 2019

Alright I will replace _accessor = frozenset() with _accessor = set() in generic.NDFrame too. Or should I just change the type to Set[str]

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@68dd979). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #25909   +/-   ##
=========================================
  Coverage          ?   91.56%           
=========================================
  Files             ?      175           
  Lines             ?    52776           
  Branches          ?        0           
=========================================
  Hits              ?    48322           
  Misses            ?     4454           
  Partials          ?        0
Flag Coverage Δ
#multiple 90.12% <100%> (?)
#single 41.81% <100%> (?)
Impacted Files Coverage Δ
pandas/core/generic.py 93.54% <100%> (ø)

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 68dd979...6829ccd. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25909      +/-   ##
==========================================
- Coverage   91.96%   91.95%   -0.01%     
==========================================
  Files         175      175              
  Lines       52405    52406       +1     
==========================================
- Hits        48193    48189       -4     
- Misses       4212     4217       +5
Flag Coverage Δ
#multiple 90.51% <100%> (ø) ⬆️
#single 40.73% <100%> (-0.13%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 96.9% <100%> (-0.12%) ⬇️
pandas/core/generic.py 93.54% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/util/testing.py 90.61% <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 d86553b...1b14fd0. Read the comment docs.

@WillAyd
Copy link
Member

WillAyd commented Mar 28, 2019

@vaibhavhrt yes change both

@vaibhavhrt
Copy link
Contributor Author

I have already changed it to _accessor = set() # type: Set[str] in both files, but it's on my other laptop. I will push them tomorrow morning.

@WillAyd WillAyd added this to the 0.25.0 milestone Mar 29, 2019
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm @jreback

@jreback
Copy link
Contributor

jreback commented Mar 30, 2019

also pls merge master

@vaibhavhrt
Copy link
Contributor Author

@jreback Yeah I saw the conflicts, looks like master updated after I started working on it. I will take care of that too.

@vaibhavhrt
Copy link
Contributor Author

@jreback I have made changes as requested. Please review it when you find some time.

@WillAyd
Copy link
Member

WillAyd commented Apr 8, 2019 via email

@WillAyd
Copy link
Member

WillAyd commented Apr 15, 2019

@vaibhavhrt can you merge master?

@vaibhavhrt
Copy link
Contributor Author

@WillAyd master merged.

@jreback jreback merged commit fd1c3f8 into pandas-dev:master Apr 16, 2019
@jreback
Copy link
Contributor

jreback commented Apr 16, 2019

thanks @vaibhavhrt

@vaibhavhrt vaibhavhrt deleted the Fix-Type-Annotations branch April 16, 2019 12:20
@vaibhavhrt
Copy link
Contributor Author

Thanks for merging, I will pick up some new issues tomorrow.

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.

Fix Type Annotations in pandas.core.generic

4 participants