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

BUG/TST/REF: Datetimelike Arithmetic Methods #23215

Merged
merged 49 commits into from
Oct 28, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
dc2280f
Make arithmetic code dispatch less redundant, fix datetime64 addition…
jbrockmendel Oct 17, 2018
37728ff
remove unnecessary nat_new
jbrockmendel Oct 17, 2018
15ad0a6
Fix interpretation of NaT
jbrockmendel Oct 17, 2018
94f1745
move test
jbrockmendel Oct 17, 2018
bf5e2fb
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 17, 2018
3bdf104
whatsnew, fix dropped timezone
jbrockmendel Oct 17, 2018
982ea30
remove comment
jbrockmendel Oct 17, 2018
d046038
Add GH references
jbrockmendel Oct 17, 2018
a0c1a85
remove unused imports
jbrockmendel Oct 18, 2018
33d82b3
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 18, 2018
8a7c249
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 18, 2018
8e57fd8
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 18, 2018
b932121
dummy commit to force CI
jbrockmendel Oct 18, 2018
9f3b18d
Fix bug in adding DateOffset to PeriodIndex, Series, Frame
jbrockmendel Oct 19, 2018
7a8232e
Dummy commit to force CI
jbrockmendel Oct 19, 2018
a743f74
revert tracebackhide
jbrockmendel Oct 19, 2018
0693196
Merge branch 'arith' of https://github.com/jbrockmendel/pandas into a…
jbrockmendel Oct 19, 2018
29d91af
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 19, 2018
af4872e
comment and reversion
jbrockmendel Oct 19, 2018
6707032
revert to simpler version
jbrockmendel Oct 20, 2018
18ef26d
Move overriding of addsub_int_array to PeriodArray
jbrockmendel Oct 20, 2018
60f7f8d
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 23, 2018
9372423
correct docstrings
jbrockmendel Oct 23, 2018
2a6268e
comments and assertions
jbrockmendel Oct 23, 2018
777f4d9
More explicit names for array/scalar add/sub methods
jbrockmendel Oct 23, 2018
ee885c8
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 23, 2018
5f231b2
oo optimization fixup
jbrockmendel Oct 23, 2018
0214a9e
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 23, 2018
fbee9f5
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 24, 2018
f7cf3d9
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 24, 2018
d799d8e
Dummy commit to force CI
jbrockmendel Oct 25, 2018
82df39c
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 25, 2018
8cf614b
change default fill_value for maybe_mask_results
jbrockmendel Oct 25, 2018
a45734a
Merge branch 'arith' of https://github.com/jbrockmendel/pandas into a…
jbrockmendel Oct 25, 2018
fb007cb
Make docstring extra explicit
jbrockmendel Oct 25, 2018
1d3fd48
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 25, 2018
aecfef7
prettify docstrings
jbrockmendel Oct 25, 2018
dade955
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 26, 2018
a6eb01c
fixup super usage
jbrockmendel Oct 26, 2018
acf1f74
flesh out TODO comment
jbrockmendel Oct 26, 2018
e89e3ef
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 26, 2018
d306277
move test for moved method
jbrockmendel Oct 26, 2018
f6e4073
Fixup name
jbrockmendel Oct 26, 2018
9146a72
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 27, 2018
4e4b9ed
change NotImplementedError to TypeError
jbrockmendel Oct 27, 2018
f35b3b6
Fix add_delta_tdi return type; docstrings
jbrockmendel Oct 27, 2018
87e07ef
Merge branch 'arith' of https://github.com/jbrockmendel/pandas into a…
jbrockmendel Oct 27, 2018
343ee30
Merge branch 'master' of https://github.com/pandas-dev/pandas into arith
jbrockmendel Oct 28, 2018
0466b9c
docstring edit, rename
jbrockmendel Oct 28, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,8 @@ Timedelta
- Bug in :class:`TimedeltaIndex` incorrectly allowing indexing with ``Timestamp`` object (:issue:`20464`)
- Fixed bug where subtracting :class:`Timedelta` from an object-dtyped array would raise ``TypeError`` (:issue:`21980`)
- Fixed bug in adding a :class:`DataFrame` with all-`timedelta64[ns]` dtypes to a :class:`DataFrame` with all-integer dtypes returning incorrect results instead of raising ``TypeError`` (:issue:`22696`)
-
- Bug in :class:`TimedeltaIndex` where adding a timezone-aware datetime scalar incorrectly returned a timezone-naive :class:`DatetimeIndex` (:issue:`23215`)
- Bug in :class:`TimedeltaIndex` where adding ``np.timedelta64('NaT')`` incorrectly returned an all-`NaT` :class:`DatetimeIndex` instead of an all-`NaT` :class:`TimedeltaIndex` (:issue:`23215`)

Timezones
^^^^^^^^^
Expand Down
80 changes: 49 additions & 31 deletions pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,27 +246,6 @@ def _maybe_mask_results(self, result, fill_value=None, convert=None):
result[self._isnan] = fill_value
return result

def _nat_new(self, box=True):
"""
Return Array/Index or ndarray filled with NaT which has the same
length as the caller.

Parameters
----------
box : boolean, default True
- If True returns a Array/Index as the same as caller.
- If False returns ndarray of np.int64.
"""
result = np.zeros(len(self), dtype=np.int64)
result.fill(iNaT)
if not box:
return result

attribs = self._get_attributes_dict()
if not is_period_dtype(self):
attribs['freq'] = None
return self._simple_new(result, **attribs)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is only used once outside of tests. Much less verbose to inline.

Copy link
Contributor

Choose a reason for hiding this comment

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

hah I just made a comment on PeriodArray about re-using this.


# ------------------------------------------------------------------
# Frequency Properties/Methods

Expand Down Expand Up @@ -347,21 +326,59 @@ def _validate_frequency(cls, index, freq, **kwargs):
# Arithmetic Methods

def _add_datelike(self, other):
# Overriden by TimedeltaArray
raise TypeError("cannot add {cls} and {typ}"
.format(cls=type(self).__name__,
typ=type(other).__name__))

_add_datelike_dti = _add_datelike
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add either doc-strings or some expl here of what is going on here. Are you overridng _dti in the sub-classes? this seems like adding a lot of verbiage here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you overridng _dti in the sub-classes?

Yes


def _sub_datelike(self, other):
Copy link
Contributor

Choose a reason for hiding this comment

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

after reading your other comments, I guess the _dti are simply the array-like versions? if so, then these really really need to be more explicit, e.g. something like

_add_datetimelike_array
_add_datelike_scalar

if not, then the others needs to be very very clear via comments & types

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 guess the _dti are simply the array-like versions?

Yes. There is an existing _add_delta_tdi, so this followed that pattern. Making the names more explicit sounds good to me.

raise com.AbstractMethodError(self)
# Overridden by DatetimeArray
assert other is not NaT
raise TypeError("cannot subtract a datelike from a {cls}"
.format(cls=type(self).__name__))

_sub_datelike_dti = _sub_datelike

def _sub_period(self, other):
return NotImplemented
# Overriden by PeriodArray
raise TypeError("cannot subtract Period from a {cls}"
.format(cls=type(self).__name__))
jorisvandenbossche marked this conversation as resolved.
Show resolved Hide resolved

def _add_offset(self, offset):
raise com.AbstractMethodError(self)

def _add_delta(self, other):
return NotImplemented
"""
Add a timedelta-like, DateOffset, or TimedeltaIndex-like object
to self.

Parameters
----------
delta : {timedelta, np.timedelta64, DateOffset,
TimedeltaIndex, ndarray[timedelta64]}

Returns
-------
result : same type as self
Copy link
Member

Choose a reason for hiding this comment

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

returns integers?

Copy link
Member Author

Choose a reason for hiding this comment

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

See comment immediately below the docstring. The return type is inaccurate in DatetimeLikeArrayMixin, but accurate in the subclasses.


Notes
-----
The result's name is set outside of _add_delta by the calling
method (__add__ or __sub__), if necessary (i.e. for Indexes).
"""
# Note: The docstring here says the return type is the same type
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 a very confusing note here. As a reader I wouldn't know what to make of it. I would just list the possible return values here, or say same as sub-class type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will make this explicit.

# as self, which is inaccurate. Once wrapped by the inheriting
# Array classes, this will be accurate.

if isinstance(other, (Tick, timedelta, np.timedelta64)):
new_values = self._add_delta_td(other)
elif is_timedelta64_dtype(other):
# ndarray[timedelta64] or TimedeltaArray/index
new_values = self._add_delta_tdi(other)

return new_values

def _add_delta_td(self, other):
"""
Expand All @@ -371,16 +388,15 @@ def _add_delta_td(self, other):
inc = delta_to_nanoseconds(other)
new_values = checked_add_with_arr(self.asi8, inc,
arr_mask=self._isnan).view('i8')
if self.hasnans:
new_values[self._isnan] = iNaT
new_values = self._maybe_mask_results(new_values, fill_value=iNaT)
Copy link
Contributor

Choose a reason for hiding this comment

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

why doesn't _maybe_mask_results have an explict iNaT as the fill value? that is almost always the appropriate one, relying on subclass writer to be correct about this seems wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea. I know there is a case where we pass NaT instead of iNaT. I think eventually we'll want to extend that method to handle this conceptually similar pattern which comes up a few times:

        if self.hasnans or other.hasnans:
            mask = (self._isnan) | (other._isnan)
            new_values[mask] = iNaT

Copy link
Contributor

Choose a reason for hiding this comment

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

i would just change the default for now (or make it None and just fill with iNaT). i don't passing this directly (and IIRC it was never this way)

Copy link
Contributor

Choose a reason for hiding this comment

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

can you change this

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

return new_values.view('i8')

def _add_delta_tdi(self, other):
"""
Add a delta of a TimedeltaIndex
return the i8 result view
"""
if not len(self) == len(other):
if len(self) != len(other):
raise ValueError("cannot add indices of unequal length")

if isinstance(other, np.ndarray):
Expand All @@ -407,7 +423,9 @@ def _add_nat(self):

# GH#19124 pd.NaT is treated like a timedelta for both timedelta
# and datetime dtypes
return self._nat_new(box=True)
result = np.zeros(len(self), dtype=np.int64)
result.fill(iNaT)
return self._shallow_copy(result, freq=None)

def _sub_nat(self):
"""Subtract pd.NaT from self"""
Expand Down Expand Up @@ -441,7 +459,7 @@ def _sub_period_array(self, other):
.format(dtype=other.dtype,
cls=type(self).__name__))

if not len(self) == len(other):
if len(self) != len(other):
raise ValueError("cannot subtract arrays/indices of "
"unequal length")
if self.freq != other.freq:
Expand Down Expand Up @@ -635,7 +653,7 @@ def __add__(self, other):
result = self._addsub_offset_array(other, operator.add)
elif is_datetime64_dtype(other) or is_datetime64tz_dtype(other):
# DatetimeIndex, ndarray[datetime64]
return self._add_datelike(other)
return self._add_datelike_dti(other)
elif is_integer_dtype(other):
result = self._addsub_int_array(other, operator.add)
elif is_float_dtype(other):
Expand Down Expand Up @@ -695,7 +713,7 @@ def __sub__(self, other):
result = self._addsub_offset_array(other, operator.sub)
elif is_datetime64_dtype(other) or is_datetime64tz_dtype(other):
# DatetimeIndex, ndarray[datetime64]
result = self._sub_datelike(other)
result = self._sub_datelike_dti(other)
elif is_period_dtype(other):
# PeriodIndex
result = self._sub_period_array(other)
Expand Down
98 changes: 31 additions & 67 deletions pandas/core/arrays/datetimes.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
from datetime import datetime, timedelta, time
from datetime import datetime, time
import warnings

import numpy as np
Expand All @@ -12,7 +12,7 @@
conversion, fields, timezones,
resolution as libresolution)

from pandas.util._decorators import cache_readonly
from pandas.util._decorators import cache_readonly, Appender
from pandas.errors import PerformanceWarning
from pandas import compat

Expand All @@ -21,7 +21,6 @@
is_object_dtype,
is_datetime64tz_dtype,
is_datetime64_dtype,
is_timedelta64_dtype,
ensure_int64)
from pandas.core.dtypes.dtypes import DatetimeTZDtype
from pandas.core.dtypes.missing import isna
Expand Down Expand Up @@ -425,10 +424,20 @@ def _assert_tzawareness_compat(self, other):
# Arithmetic Methods

def _sub_datelike_dti(self, other):
"""subtraction of two DatetimeIndexes"""
if not len(self) == len(other):
"""subtract DatetimeArray/Index or ndarray[datetime64]"""
if len(self) != len(other):
raise ValueError("cannot add indices of unequal length")

if isinstance(other, np.ndarray):
jorisvandenbossche marked this conversation as resolved.
Show resolved Hide resolved
# if other is an ndarray, we assume it is datetime64-dtype
other = type(self)(other)

if not self._has_same_tz(other):
# require tz compat
raise TypeError("{cls} subtraction must have the same "
"timezones or no timezones"
.format(cls=type(self).__name__))

self_i8 = self.asi8
other_i8 = other.asi8
new_values = checked_add_with_arr(self_i8, -other_i8,
Expand Down Expand Up @@ -458,72 +467,27 @@ def _add_offset(self, offset):

def _sub_datelike(self, other):
# subtract a datetime from myself, yielding a ndarray[timedelta64[ns]]
if isinstance(other, (DatetimeArrayMixin, np.ndarray)):
if isinstance(other, np.ndarray):
# if other is an ndarray, we assume it is datetime64-dtype
other = type(self)(other)
if not self._has_same_tz(other):
# require tz compat
raise TypeError("{cls} subtraction must have the same "
"timezones or no timezones"
.format(cls=type(self).__name__))
result = self._sub_datelike_dti(other)
elif isinstance(other, (datetime, np.datetime64)):
assert other is not NaT
other = Timestamp(other)
if other is NaT:
return self - NaT
assert isinstance(other, (datetime, np.datetime64))
jorisvandenbossche marked this conversation as resolved.
Show resolved Hide resolved
assert other is not NaT
other = Timestamp(other)
if other is NaT:
return self - NaT

if not self._has_same_tz(other):
# require tz compat
elif not self._has_same_tz(other):
raise TypeError("Timestamp subtraction must have the same "
"timezones or no timezones")
else:
i8 = self.asi8
result = checked_add_with_arr(i8, -other.value,
arr_mask=self._isnan)
result = self._maybe_mask_results(result,
fill_value=iNaT)
else:
raise TypeError("cannot subtract {cls} and {typ}"
.format(cls=type(self).__name__,
typ=type(other).__name__))
raise TypeError("Timestamp subtraction must have the same "
"timezones or no timezones")

i8 = self.asi8
result = checked_add_with_arr(i8, -other.value,
arr_mask=self._isnan)
result = self._maybe_mask_results(result, fill_value=iNaT)
return result.view('timedelta64[ns]')

@Appender(dtl.DatetimeLikeArrayMixin._add_delta.__doc__)
def _add_delta(self, delta):
"""
Add a timedelta-like, DateOffset, or TimedeltaIndex-like object
to self.

Parameters
----------
delta : {timedelta, np.timedelta64, DateOffset,
TimedeltaIndex, ndarray[timedelta64]}

Returns
-------
result : same type as self

Notes
-----
The result's name is set outside of _add_delta by the calling
method (__add__ or __sub__)
"""
from pandas.core.arrays import TimedeltaArrayMixin

if isinstance(delta, (Tick, timedelta, np.timedelta64)):
new_values = self._add_delta_td(delta)
elif is_timedelta64_dtype(delta):
if not isinstance(delta, TimedeltaArrayMixin):
delta = TimedeltaArrayMixin(delta)
new_values = self._add_delta_tdi(delta)
else:
new_values = self.astype('O') + delta

tz = 'UTC' if self.tz is not None else None
result = type(self)(new_values, tz=tz, freq='infer')
if self.tz is not None and self.tz is not utc:
result = result.tz_convert(self.tz)
return result
new_values = dtl.DatetimeLikeArrayMixin._add_delta(self, delta)
return type(self)(new_values, tz=self.tz, freq='infer')
jbrockmendel marked this conversation as resolved.
Show resolved Hide resolved

# -----------------------------------------------------------------
# Timezone Conversion and Localization Methods
Expand Down
40 changes: 7 additions & 33 deletions pandas/core/arrays/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
from pandas._libs.tslibs.fields import isleapyear_arr

from pandas import compat
from pandas.util._decorators import (cache_readonly, deprecate_kwarg)
from pandas.util._decorators import cache_readonly, deprecate_kwarg, Appender

from pandas.core.dtypes.common import (
is_integer_dtype, is_float_dtype, is_period_dtype, is_timedelta64_dtype,
is_integer_dtype, is_float_dtype, is_period_dtype,
is_datetime64_dtype, _TD_DTYPE)
from pandas.core.dtypes.dtypes import PeriodDtype
from pandas.core.dtypes.generic import ABCSeries
Expand Down Expand Up @@ -334,10 +334,6 @@ def to_timestamp(self, freq=None, how='start'):

_create_comparison_method = classmethod(_period_array_cmp)

def _sub_datelike(self, other):
assert other is not NaT
return NotImplemented

def _sub_period(self, other):
# If the operation is well-defined, we return an object-Index
# of DateOffsets. Null entries are filled with pd.NaT
Expand All @@ -349,9 +345,7 @@ def _sub_period(self, other):
new_data = asi8 - other.ordinal
new_data = np.array([self.freq * x for x in new_data])

if self.hasnans:
new_data[self._isnan] = NaT

new_data = self._maybe_mask_results(new_data, fill_value=NaT)
return new_data

def _add_offset(self, other):
Expand Down Expand Up @@ -379,38 +373,18 @@ def _add_delta_tdi(self, other):
delta = self._check_timedeltalike_freq_compat(other)
return self._addsub_int_array(delta, operator.add)

@Appender(DatetimeLikeArrayMixin._add_delta.__doc__)
Copy link
Contributor

@TomAugspurger TomAugspurger Oct 19, 2018

Choose a reason for hiding this comment

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

Why this? Doesn't PeriodArrayMixin inherit directly from DatetimeLikeArrayMixin? The docstring will be inherited.

Copy link
Member Author

Choose a reason for hiding this comment

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

The docstring will be inherited.

Will it? I thought that required some metaclass hijinx

Copy link
Contributor

Choose a reason for hiding this comment

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

In [1]: import pandas as pd

In [2]: class A:
   ...:     def a(self):
   ...:         """doc"""
   ...:         pass
   ...:

In [3]: class B(A):
   ...:     pass
   ...:

In [4]: B.a?
Signature: B.a(self)
Docstring: doc
File:      ~/sandbox/pandas/<ipython-input-2-6d9a0c65a8ca>
Type:      function

although maybe something else gets in the way?

Copy link
Member Author

Choose a reason for hiding this comment

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

The distinction is that we actually are overriding the method

Copy link
Contributor

Choose a reason for hiding this comment

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

On this branch with

diff --git a/pandas/core/arrays/period.py b/pandas/core/arrays/period.py
index 1d7b57801..ff297e5bd 100644
--- a/pandas/core/arrays/period.py
+++ b/pandas/core/arrays/period.py
@@ -378,7 +378,6 @@ class PeriodArrayMixin(DatetimeLikeArrayMixin):
         delta = self._check_timedeltalike_freq_compat(other)
         return self._addsub_int_array(delta, operator.add)
 
-    @Appender(DatetimeLikeArrayMixin._add_delta.__doc__)
     def _add_delta(self, other):
         if not isinstance(self.freq, Tick):
             # We cannot add timedelta-like to non-tick PeriodArray

the docstring looks fine.

In [3]: pd.core.arrays.PeriodArrayMixin._add_delta?
Signature: pd.core.arrays.PeriodArrayMixin._add_delta(self, other)
Docstring:
Add a timedelta-like, Tick or TimedeltaIndex-like object
to self.

Parameters
----------
delta : {timedelta, np.timedelta64, Tick,
         TimedeltaIndex, ndarray[timedelta64]}

Returns
-------
result : same type as self

Notes
-----
The result's name is set outside of _add_delta by the calling
method (__add__ or __sub__), if necessary (i.e. for Indexes).
File:      ~/sandbox/pandas/pandas/core/arrays/period.py
Type:      function

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, for a final check I ran it through sphinx

python make.py --single=pandas.core.arrays.PeriodArrayMixin._add_delta

and it does whatever IPython is doing. The rendered docstring looks fine to.

In conclusion: I think we can remove the Appender.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there some cost I'm missing to having print(PeriodArrayMixin._add_delta.__doc__) work correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a common use case? I assume most people use help() or ?.

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 usually print the docstring, mostly because I usually forget about the other usages. This is an internal method so it doesn't really matter. My view ATM is just that its zero-cost.

Copy link
Member

Choose a reason for hiding this comment

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

Same comment here about a single line comment summarizing what it does

def _add_delta(self, other):
"""
Add a timedelta-like, Tick, or TimedeltaIndex-like object
to self.

Parameters
----------
other : {timedelta, np.timedelta64, Tick,
TimedeltaIndex, ndarray[timedelta64]}

Returns
-------
result : same type as self
"""
if not isinstance(self.freq, Tick):
# We cannot add timedelta-like to non-tick PeriodArray
raise IncompatibleFrequency("Input has different freq from "
"{cls}(freq={freqstr})"
.format(cls=type(self).__name__,
freqstr=self.freqstr))

# TODO: standardize across datetimelike subclasses whether to return
# i8 view or _shallow_copy
if isinstance(other, (Tick, timedelta, np.timedelta64)):
new_values = self._add_delta_td(other)
return self._shallow_copy(new_values)
elif is_timedelta64_dtype(other):
# ndarray[timedelta64] or TimedeltaArray/index
new_values = self._add_delta_tdi(other)
return self._shallow_copy(new_values)
else: # pragma: no cover
raise TypeError(type(other).__name__)
new_values = DatetimeLikeArrayMixin._add_delta(self, other)

return self._shallow_copy(new_values)

@deprecate_kwarg(old_arg_name='n', new_arg_name='periods')
def shift(self, periods):
Expand Down
Loading