-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
move ndarray-returning functions to EA mixin classes #21722
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
Conversation
Hello @jbrockmendel! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on July 04, 2018 at 17:06 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #21722 +/- ##
==========================================
+ Coverage 91.92% 91.93% +<.01%
==========================================
Files 158 158
Lines 49702 49716 +14
==========================================
+ Hits 45690 45704 +14
Misses 4012 4012
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. doc comments
pandas/core/arrays/period.py
Outdated
# ------------------------------------------------------------------ | ||
# Arithmetic Methods | ||
|
||
def _sub_datelike(self, other): | ||
assert other is not NaT | ||
return NotImplemented | ||
|
||
def _maybe_convert_timedelta(self, other): | ||
if isinstance( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a doc-string to this one as it accepts many types
lgtm. ping on green. |
ping |
thanks! |
Orthogonal to other EAMixin PRs.
The methods this moves are unchanged. Docstrings changed e.g. "DatetimeIndex" to "Datetime Array/Index". I'm open to suggestions for better wording.
The error message in
PeriodArrayMixin._maybe_convert_timedelta
is changed to usetype(self).__name__
instead of hardcodingPeriodIndex
. Ditto the warning issued byPeriodArrayMixin.freq.setter
.