Skip to content

Use ._tshift internally for datetimelike ops #22949

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 5, 2018
Merged

Use ._tshift internally for datetimelike ops #22949

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23e5cfc
Use ._tshift internally for datetimelike ops
TomAugspurger Oct 2, 2018
c3a96d0
Merge remote-tracking branch 'upstream/master' into datetimelike-tshift
TomAugspurger Oct 3, 2018
67faabc
rename to time_shift
TomAugspurger Oct 3, 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
23 changes: 20 additions & 3 deletions pandas/core/arrays/datetimelike.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ def _sub_period_array(self, other):
def _addsub_int_array(self, other, op):
"""
Add or subtract array-like of integers equivalent to applying
`shift` pointwise.
`_tshift` pointwise.

Parameters
----------
Expand Down Expand Up @@ -553,6 +553,23 @@ def shift(self, periods, freq=None):
--------
Index.shift : Shift values of Index.
"""
return self._tshift(periods=periods, freq=freq)

def _tshift(self, periods, freq=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't we call this ._time_shift or ._period_shift

Copy link
Contributor

Choose a reason for hiding this comment

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

if you are actually going to change this, then this should be much more consistent across the code base, e.g. does internals need changing, what about the Index classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose _tshift to match Series.tshift, which has the same behavior, but for the Series' index.

Copy link
Contributor

Choose a reason for hiding this comment

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

we have a note to deprecate tshift entirely. I don't think this is a good name at all.

"""
Shift each value by `periods`.

Note this is different from ExtensionArray.shift, which
Copy link
Contributor

Choose a reason for hiding this comment

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

how is this relevant here? this is simply a re-definition of what shift means. I understand the desire to make things clear, but this has always been the case, for example in comparing a DTI to an Index.

shifts the *position* of each element, padding the end with
missing values.

Parameters
----------
periods : int
Number of periods to shift by.
freq : pandas.DateOffset, pandas.Timedelta, or string
Frequency increment to shift by.
"""
if freq is not None and freq != self.freq:
if isinstance(freq, compat.string_types):
freq = frequencies.to_offset(freq)
Expand Down Expand Up @@ -600,7 +617,7 @@ def __add__(self, other):
elif lib.is_integer(other):
# This check must come after the check for np.timedelta64
# as is_integer returns True for these
result = self.shift(other)
result = self._tshift(other)

# array-like others
elif is_timedelta64_dtype(other):
Expand Down Expand Up @@ -652,7 +669,7 @@ def __sub__(self, other):
elif lib.is_integer(other):
# This check must come after the check for np.timedelta64
# as is_integer returns True for these
result = self.shift(-other)
result = self._tshift(-other)
elif isinstance(other, Period):
result = self._sub_period(other)

Expand Down
9 changes: 6 additions & 3 deletions pandas/core/arrays/period.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def _add_offset(self, other):
if base != self.freq.rule_code:
msg = DIFFERENT_FREQ_INDEX.format(self.freqstr, other.freqstr)
raise IncompatibleFrequency(msg)
return self.shift(other.n)
return self._tshift(other.n)

def _add_delta_td(self, other):
assert isinstance(other, (timedelta, np.timedelta64, Tick))
Expand All @@ -307,7 +307,7 @@ def _add_delta_td(self, other):
if isinstance(own_offset, Tick):
offset_nanos = delta_to_nanoseconds(own_offset)
if np.all(nanos % offset_nanos == 0):
return self.shift(nanos // offset_nanos)
return self._tshift(nanos // offset_nanos)

# raise when input doesn't have freq
raise IncompatibleFrequency("Input has different freq from "
Expand All @@ -317,7 +317,7 @@ def _add_delta_td(self, other):

def _add_delta(self, other):
ordinal_delta = self._maybe_convert_timedelta(other)
return self.shift(ordinal_delta)
return self._tshift(ordinal_delta)

def shift(self, n):
"""
Expand All @@ -332,6 +332,9 @@ def shift(self, n):
-------
shifted : Period Array/Index
"""
return self._tshift(n)

def _tshift(self, n):
values = self._ndarray_values + n * self.freq.n
if self.hasnans:
values[self._isnan] = iNaT
Expand Down