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

API: Simplify repeat signature #24447

Merged
merged 1 commit into from
Dec 27, 2018
Merged

Conversation

jschendel
Copy link
Member

Doesn't seem like a whatsnew entry is needed here as the behavior from a user perspective should remain the same.

cc @TomAugspurger

@jschendel jschendel added API Design ExtensionArray Extending pandas with custom dtypes or arrays. labels Dec 26, 2018
@jschendel jschendel added this to the 0.24.0 milestone Dec 26, 2018
def repeat(self, repeats, *args, **kwargs):
nv.validate_repeat(args, kwargs)
def repeat(self, repeats, axis=None):
nv.validate_repeat(tuple(), dict(axis=axis))
Copy link
Member Author

Choose a reason for hiding this comment

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

I think these could be written more explicitly as:

if axis is not None:
    raise ValueError('...')

I don't think nv.validate_repeat is doing more than that but could be missing something. The advantage to using nv.validate_repeat simply being that the error message is kept in a single place instead of being explicitly written in each method.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is exactly why we have it this way

@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

lgtm. merge on green.

@codecov
Copy link

codecov bot commented Dec 26, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24447      +/-   ##
==========================================
+ Coverage    92.3%    92.3%   +<.01%     
==========================================
  Files         163      163              
  Lines       51977    51983       +6     
==========================================
+ Hits        47979    47985       +6     
  Misses       3998     3998
Flag Coverage Δ
#multiple 90.72% <100%> (ø) ⬆️
#single 43% <50%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/series.py 93.72% <100%> (ø) ⬆️
pandas/core/arrays/base.py 97.6% <100%> (+0.01%) ⬆️
pandas/core/indexes/multi.py 95.58% <100%> (ø) ⬆️
pandas/core/indexes/datetimelike.py 97.59% <100%> (ø) ⬆️
pandas/core/arrays/categorical.py 95.34% <100%> (ø) ⬆️
pandas/core/indexes/base.py 96.28% <100%> (ø) ⬆️
pandas/core/arrays/interval.py 93.04% <100%> (+0.02%) ⬆️

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 8b4b38f...62f040d. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Dec 26, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24447      +/-   ##
==========================================
+ Coverage    92.3%    92.3%   +<.01%     
==========================================
  Files         163      163              
  Lines       51977    51983       +6     
==========================================
+ Hits        47979    47985       +6     
  Misses       3998     3998
Flag Coverage Δ
#multiple 90.72% <100%> (ø) ⬆️
#single 43% <50%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/series.py 93.72% <100%> (ø) ⬆️
pandas/core/arrays/base.py 97.6% <100%> (+0.01%) ⬆️
pandas/core/indexes/multi.py 95.58% <100%> (ø) ⬆️
pandas/core/indexes/datetimelike.py 97.59% <100%> (ø) ⬆️
pandas/core/arrays/categorical.py 95.34% <100%> (ø) ⬆️
pandas/core/indexes/base.py 96.28% <100%> (ø) ⬆️
pandas/core/arrays/interval.py 93.04% <100%> (+0.02%) ⬆️

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 8b4b38f...62f040d. Read the comment docs.

@jreback jreback merged commit 9ed49eb into pandas-dev:master Dec 27, 2018
@jreback
Copy link
Contributor

jreback commented Dec 27, 2018

thanks @jschendel

@jschendel jschendel deleted the repeat-signature branch December 27, 2018 01:12
thoo added a commit to thoo/pandas that referenced this pull request Dec 28, 2018
* upstream/master: (26 commits)
  DOC: Fixing doc upload (no such remote origin) (pandas-dev#24459)
  BLD: for C extension builds on mac, target macOS 10.9 where possible (pandas-dev#24274)
  POC: _eadata (pandas-dev#24394)
  DOC: Correct location (pandas-dev#24456)
  CI: Moving CircleCI build to Travis (pandas-dev#24449)
  BUG: Infer compression by default in read_fwf() (pandas-dev#22200)
  DOC: Fix minor typo in whatsnew (pandas-dev#24453)
  DOC: Add dateutil to intersphinx pandas-dev#24437 (pandas-dev#24443)
  DOC: Adding links to offset classes in timeseries.rst (pandas-dev#24448)
  DOC: Adding offsets to API ref (pandas-dev#24446)
  DOC: fix flake8 issue in groupby.rst (pandas-dev#24363)
  DOC: Fixing more doc warnings (pandas-dev#24438)
  API: Simplify repeat signature (pandas-dev#24447)
  BUG: to_datetime(Timestamp, utc=True) localizes to UTC (pandas-dev#24441)
  CLN: Cython Py2/3 Compatible Imports (pandas-dev#23940)
  DOC: Fixing more doc warnings (pandas-dev#24431)
  DOC: Removing old release.rst (pandas-dev#24427)
  BUG-24408 Series.dt does not maintain own copy of index (pandas-dev#24426)
  DOC: Fixing several doc warnings (pandas-dev#24430)
  ENH: fill_value argument for shift pandas-dev#15486 (pandas-dev#24128)
  ...
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design ExtensionArray Extending pandas with custom dtypes or arrays.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExtensionArray/Index/Series.repeat signature
2 participants