Skip to content

INT: take DateOffset out of the inheritance tree for BaseOffset subclasses #34147

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 11 commits into from
May 13, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions pandas/_libs/tslibs/offsets.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,11 @@ class _BaseOffset:
)
return self.is_anchored()

def is_anchored(self) -> bool:
# TODO: Does this make sense for the general case? It would help
# if there were a canonical docstring for what is_anchored means.
return self.n == 1


class BaseOffset(_BaseOffset):
# Here we add __rfoo__ methods that don't play well with cdef classes
Expand Down
2 changes: 1 addition & 1 deletion pandas/tseries/frequencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def _get_offset(name: str) -> DateOffset:
klass = prefix_mapping[split[0]]
# handles case where there's no suffix (and will TypeError if too
# many '-')
offset = klass._from_name(*split[1:])
offset = klass._from_name(*split[1:]) # type: ignore
except (ValueError, TypeError, KeyError) as err:
# bad prefix or suffix
raise ValueError(libfreqs.INVALID_FREQ_ERR_MSG.format(name)) from err
Expand Down
34 changes: 20 additions & 14 deletions pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,22 @@
# DateOffset


class DateOffset(BaseOffset):
class OffsetMeta(type):
"""
Metaclass that allows us to pretend that all BaseOffset subclasses
inherit from DateOffset (which is needed for backward-compatibility).
"""

@classmethod
def __instancecheck__(cls, obj) -> bool:
return isinstance(obj, BaseOffset)

@classmethod
def __subclasscheck__(cls, obj) -> bool:
Copy link
Member Author

Choose a reason for hiding this comment

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

@simonjayhawkins is this going to mess up annotations elsewhere we annotate something as a DateOffset?

Copy link
Member

Choose a reason for hiding this comment

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

mypy is a static checker, so IIUC mypy doesn't execute the code in __instancecheck__ and __subclasscheck__. Therefore, mypy has no way of knowing that BaseOffset subclass pretend to be DateOffset subclasses.

This is probably a use case for Protocol. i.e. structural typing instead of nominal typing.

return issubclass(obj, BaseOffset)


class DateOffset(BaseOffset, metaclass=OffsetMeta):
"""
Standard kind of date increment used for a date range.

Expand Down Expand Up @@ -275,25 +290,16 @@ def apply_index(self, i):
"applied vectorized"
)

def is_anchored(self) -> bool:
# TODO: Does this make sense for the general case? It would help
# if there were a canonical docstring for what is_anchored means.
return self.n == 1

def is_on_offset(self, dt):
if self.normalize and not is_normalized(dt):
return False
# TODO, see #1395
return True


class SingleConstructorOffset(DateOffset):
# All DateOffset subclasses (other than Tick) subclass SingleConstructorOffset
__init__ = BaseOffset.__init__
_attributes = BaseOffset._attributes
apply_index = BaseOffset.apply_index
is_on_offset = BaseOffset.is_on_offset
_adjust_dst = True
class SingleConstructorOffset(BaseOffset):
_params = cache_readonly(BaseOffset._params.fget)
freqstr = cache_readonly(BaseOffset.freqstr.fget)

@classmethod
def _from_name(cls, suffix=None):
Expand Down Expand Up @@ -2389,7 +2395,7 @@ def generate_range(start=None, end=None, periods=None, offset=BDay()):


prefix_mapping = {
offset._prefix: offset
offset._prefix: offset # type: ignore
for offset in [
YearBegin, # 'AS'
YearEnd, # 'A'
Expand Down