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

add missing args, make kwarg explicit #19691

Merged
merged 2 commits into from
Feb 15, 2018

Conversation

jbrockmendel
Copy link
Member

Index._evaluate_compare is never hit, but if it were it would raise twice: once for not having the right signature and again for referencing a non-existent base.AbstractMethodError

BlockManager.isna is entirely non-obvious because there is a required kwarg that isn't in its signature. This makes it explicit.

@TomAugspurger
Copy link
Contributor

@TomAugspurger TomAugspurger added Refactor Internal refactoring of code Internals Related to non-user accessible pandas implementation Clean labels Feb 14, 2018
@jbrockmendel
Copy link
Member Author

Nope. _evaluate_compare gets overriden in DatetimeIndexOpsMixin, and the only classes that woudl have needs_i8_conversion(self) get it from there.

@codecov
Copy link

codecov bot commented Feb 14, 2018

Codecov Report

Merging #19691 into master will increase coverage by 0.02%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19691      +/-   ##
==========================================
+ Coverage   91.58%    91.6%   +0.02%     
==========================================
  Files         150      150              
  Lines       48867    48867              
==========================================
+ Hits        44755    44767      +12     
+ Misses       4112     4100      -12
Flag Coverage Δ
#multiple 89.98% <75%> (+0.02%) ⬆️
#single 41.75% <75%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/internals.py 95.05% <100%> (ø) ⬆️
pandas/core/indexes/base.py 96.46% <50%> (ø) ⬆️
pandas/plotting/_converter.py 66.95% <0%> (+1.73%) ⬆️

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 d198a6e...e22bf64. Read the comment docs.

@jreback jreback added this to the 0.23.0 milestone Feb 15, 2018
@jreback jreback merged commit 2fdf1e2 into pandas-dev:master Feb 15, 2018
@jreback
Copy link
Contributor

jreback commented Feb 15, 2018

thanks!

harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018
@jbrockmendel jbrockmendel deleted the missing_kwargs branch June 22, 2018 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Internals Related to non-user accessible pandas implementation Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants