Skip to content

REF: Refactor Datetimelike delegation #24039

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

Merged
merged 7 commits into from
Dec 3, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
47 changes: 47 additions & 0 deletions pandas/core/indexes/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
Base and utility classes for tseries type pandas objects.
"""
import operator
import warnings

import numpy as np
Expand All @@ -22,6 +23,7 @@
from pandas.core.dtypes.missing import isna

from pandas.core import algorithms, ops
from pandas.core.accessor import PandasDelegate
from pandas.core.arrays import PeriodArray
from pandas.core.arrays.datetimelike import DatetimeLikeArrayMixin
import pandas.core.indexes.base as ibase
Expand Down Expand Up @@ -855,3 +857,48 @@ def f(self):
f.__name__ = fget.__name__
f.__doc__ = fget.__doc__
return property(f)


class DatetimelikeDelegateMixin(PandasDelegate):
Copy link
Member

Choose a reason for hiding this comment

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

Is PandasDelegate overkill for this situation? It's not like Series.dt where we construct an object and need to cache it. Couldn't we just use something like:

def pass_through_methods(names):
    def decorator(cls):
        for name in names:
            method = wrap_array_method(...)
            setattr(cls, name, method)
        return cls

There's definitely overlap with the PandasDelegate machinery, but I think that machinery makes for really dense reading, so should be reserved for when we really need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps you're thinking of Properties (which subclasses PandasDelegate)? PandasDelegate doesn't have an __init__, it just handles registering and creating the delegated properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But, I don't have a strong preference of one of the other. We use PandasDelegate on master, for period, so this PR is a strict generalization of that.

"""
Delegation mechanism, specific for Datetime, Timedelta, and Period types.

Functionality is delegated from the Index class to an Array class. A
few things can be customized

* _delegate_class : type
The class being delegated to.
* _delegated_methods, delegated_properties : List
The list of property / method names being delagated.
* raw_methods : Set
The set of methods whose results should should *not* be
boxed in an index, after being returned from the array
* raw_properties : Set
The set of properties whose results should should *not* be
boxed in an index, after being returned from the array
"""
# raw_methods : dispatch methods that shouldn't be boxed in an Index
_raw_methods = set()
# raw_properties : dispatch properties that shouldn't be boxed in an Index
_raw_properties = set()
name = None
_data = None

@property
def _delegate_class(self):
raise AbstractMethodError

def _delegate_property_get(self, name, *args, **kwargs):
result = getattr(self._data, name)
if name not in self._raw_properties:
result = Index(result, name=self.name)
return result

def _delegate_property_set(self, name, value, *args, **kwargs):
setattr(self._data, name, value)

def _delegate_method(self, name, *args, **kwargs):
result = operator.methodcaller(name, *args, **kwargs)(self._data)
if name not in self._raw_methods:
result = Index(result, name=self.name)
Copy link
Member

Choose a reason for hiding this comment

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

this will need checks for 2-tuples in divmod and rdivmod, for scalars for eventual reduction ops

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 think all the delegated methods return a single value.

Copy link
Member

Choose a reason for hiding this comment

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

If #23885 goes through that will change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha. I'd prefer to wait until we can hit that code path before worrying about it.

Copy link
Member

Choose a reason for hiding this comment

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

Totally fair.

return result
52 changes: 13 additions & 39 deletions pandas/core/indexes/period.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# pylint: disable=E1101,E1103,W0232
from datetime import datetime, timedelta
import operator
import warnings

import numpy as np
Expand All @@ -18,15 +17,16 @@

from pandas import compat
from pandas.core import common as com
from pandas.core.accessor import PandasDelegate, delegate_names
from pandas.core.accessor import delegate_names
from pandas.core.algorithms import unique1d
import pandas.core.arrays.datetimelike as dtl
from pandas.core.arrays.period import PeriodArray, period_array
from pandas.core.base import _shared_docs
import pandas.core.indexes.base as ibase
from pandas.core.indexes.base import _index_shared_docs, ensure_index
from pandas.core.indexes.datetimelike import (
DatelikeOps, DatetimeIndexOpsMixin, wrap_arithmetic_op)
DatelikeOps, DatetimeIndexOpsMixin, DatetimelikeDelegateMixin,
wrap_arithmetic_op)
from pandas.core.indexes.datetimes import DatetimeIndex, Index, Int64Index
from pandas.core.missing import isna
from pandas.core.ops import get_op_result_name
Expand Down Expand Up @@ -54,35 +54,24 @@ def _new_PeriodIndex(cls, **d):
return cls(values, **d)


class PeriodDelegateMixin(PandasDelegate):
class PeriodDelegateMixin(DatetimelikeDelegateMixin):
"""
Delegate from PeriodIndex to PeriodArray.
"""
def _delegate_property_get(self, name, *args, **kwargs):
result = getattr(self._data, name)
box_ops = (
set(PeriodArray._datetimelike_ops) - set(PeriodArray._bool_ops)
)
if name in box_ops:
result = Index(result, name=self.name)
return result

def _delegate_property_set(self, name, value, *args, **kwargs):
setattr(self._data, name, value)

def _delegate_method(self, name, *args, **kwargs):
result = operator.methodcaller(name, *args, **kwargs)(self._data)
return Index(result, name=self.name)
_delegate_class = PeriodArray
_delegated_properties = PeriodArray._datetimelike_ops
_delegated_methods = (
set(PeriodArray._datetimelike_methods) | {'_addsub_int_array'}
)
_raw_properties = {'is_leap_year'}


@delegate_names(PeriodArray,
PeriodArray._datetimelike_ops + ['size', 'asi8', 'shape'],
PeriodDelegateMixin._delegated_properties,
typ='property')
@delegate_names(PeriodArray,
[x for x in PeriodArray._datetimelike_methods
if x not in {"asfreq", "to_timestamp"}],
typ="method",
overwrite=True)
PeriodDelegateMixin._delegated_methods,
typ="method")
class PeriodIndex(DatelikeOps, DatetimeIndexOpsMixin,
Int64Index, PeriodDelegateMixin):
"""
Expand Down Expand Up @@ -349,21 +338,6 @@ def _maybe_box_as_values(self, values, **attribs):
freq = attribs['freq']
return PeriodArray(values, freq=freq)

# ------------------------------------------------------------------------
# Dispatch and maybe box. Not done in delegate_names because we box
# different from those (which use Index).

def asfreq(self, freq=None, how='E'):
result = self._data.asfreq(freq=freq, how=how)
return self._simple_new(result, name=self.name)

def to_timestamp(self, freq=None, how='start'):
from pandas import DatetimeIndex
result = self._data.to_timestamp(freq=freq, how=how)
return DatetimeIndex._simple_new(result.asi8,
name=self.name,
freq=result.freq)

def _maybe_convert_timedelta(self, other):
"""
Convert timedelta-like input to an integer multiple of self.freq
Expand Down