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

MAINT: Drop take_last kwarg from method signatures #15710

Closed
wants to merge 1 commit into from

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Mar 17, 2017

Affected methods:

  1. nlargest
  2. nsmallest
  3. duplicated
  4. drop_duplicates

xref #10236, #10792, #10920.

@jreback jreback added the Deprecate Functionality to remove in pandas label Mar 17, 2017
@jreback jreback added this to the 0.20.0 milestone Mar 17, 2017
@codecov
Copy link

codecov bot commented Mar 17, 2017

Codecov Report

Merging #15710 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #15710      +/-   ##
==========================================
- Coverage   91.02%      91%   -0.03%     
==========================================
  Files         143      143              
  Lines       49416    49396      -20     
==========================================
- Hits        44981    44951      -30     
- Misses       4435     4445      +10
Impacted Files Coverage Δ
pandas/indexes/base.py 96.14% <ø> (-0.01%)
pandas/core/base.py 95.48% <ø> (-0.03%)
pandas/indexes/multi.py 96.59% <ø> (-0.01%)
pandas/core/series.py 94.89% <ø> (-0.02%)
pandas/core/frame.py 97.87% <100%> (-0.11%)
pandas/indexes/category.py 98.28% <100%> (-0.01%)
pandas/core/groupby.py 94.96% <100%> (-0.03%)
pandas/io/gbq.py 25% <0%> (-58.34%)
pandas/tools/merge.py 93.68% <0%> (-0.34%)
... and 1 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 5e96fb0...b416290. Read the comment docs.

@Appender(Series.nlargest.__doc__)
def nlargest(self, n=5, keep='first'):
# ToDo: When we remove deprecate_kwargs, we can remote these methods
# TODO: When we remove deprecate_kwargs, we can remove these methods
# and include nlargest and nsmallest to _series_apply_whitelist
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we do this comment?

Copy link
Member Author

@gfyoung gfyoung Mar 17, 2017

Choose a reason for hiding this comment

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

Ah yes, I should think so. Will do.

Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm otherwise. ping when ready.

_series_apply_whitelist = \
(_common_apply_whitelist - set(['boxplot'])) | \
frozenset(['dtype', 'unique'])
_series_apply_whitelist = ((_common_apply_whitelist |
Copy link
Contributor

Choose a reason for hiding this comment

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

should you actully just add them to the _common_apply_whitelist?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly, but what would it mean though in the context of a DataFrame?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh these are not defined on DataFrame, ok then.

@gfyoung
Copy link
Member Author

gfyoung commented Mar 17, 2017

Separate issue: why did so many Travis builds get canceled (including my most recent one)?

@jreback
Copy link
Contributor

jreback commented Mar 17, 2017

fixing travis so hold a bit to repush

@jreback
Copy link
Contributor

jreback commented Mar 17, 2017

caching wasn't working for miniconda. turned it back on, then then found an error, fixed it, which led to the next one......soon.

@jreback
Copy link
Contributor

jreback commented Mar 17, 2017

ok pull and try again. I think its fixed :>

Affected methods:

1) nlargest
2) nsmallest
3) duplicated
4) drop_duplicates

xref pandas-devgh-10236, pandas-devgh-10792, pandas-devgh-10920.
@jreback
Copy link
Contributor

jreback commented Mar 17, 2017

lgtm. ping on green.

@jreback jreback closed this in a9c8239 Mar 18, 2017
@jreback
Copy link
Contributor

jreback commented Mar 18, 2017

thanks!

keep em coming!

@gfyoung gfyoung deleted the create-last-kw-drop branch March 18, 2017 01:27
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
Affected methods:

1) nlargest
2) nsmallest
3) duplicated
4) drop_duplicates

xref pandas-dev#10236, pandas-dev#10792, pandas-dev#10920.

Author: gfyoung <gfyoung17@gmail.com>

Closes pandas-dev#15710 from gfyoung/create-last-kw-drop and squashes the following commits:

b416290 [gfyoung] MAINT: Drop take_last kwarg from method signatures
mattip pushed a commit to mattip/pandas that referenced this pull request Apr 3, 2017
Affected methods:

1) nlargest
2) nsmallest
3) duplicated
4) drop_duplicates

xref pandas-dev#10236, pandas-dev#10792, pandas-dev#10920.

Author: gfyoung <gfyoung17@gmail.com>

Closes pandas-dev#15710 from gfyoung/create-last-kw-drop and squashes the following commits:

b416290 [gfyoung] MAINT: Drop take_last kwarg from method signatures
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants