-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: mix PeriodPseudoDtype into PeriodDtype #34590
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
Changes from 3 commits
1b8ba42
4fac4a6
37fc127
4f598da
44a59eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
import pytz | ||
|
||
from pandas._libs.interval import Interval | ||
from pandas._libs.tslibs import NaT, Period, Timestamp, timezones, to_offset | ||
from pandas._libs.tslibs import NaT, Period, Timestamp, dtypes, timezones, to_offset | ||
from pandas._libs.tslibs.offsets import BaseOffset | ||
from pandas._typing import DtypeObj, Ordered | ||
|
||
|
@@ -848,7 +848,7 @@ def __setstate__(self, state) -> None: | |
|
||
|
||
@register_extension_dtype | ||
class PeriodDtype(PandasExtensionDtype): | ||
class PeriodDtype(dtypes.PeriodPseudoDtype, PandasExtensionDtype): | ||
""" | ||
An ExtensionDtype for Period data. | ||
|
||
|
@@ -896,7 +896,8 @@ def __new__(cls, freq=None): | |
|
||
elif freq is None: | ||
# empty constructor for pickle compat | ||
u = object.__new__(cls) | ||
# -10_000 corresponds to PeriodDtypeCode.UNDEFINED | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this actually hit in tests? what is the point of UNDEFINED? I mean, when does this become defined There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. We have a test that constructs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right, how does this get resolved though? do you see periods with undefined after this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think the idea is to use it as a placeholder until you can get a "real" PeriodDtype. e.g. if you know that you are parsing a csv column to Period but dont yet know the freq. just speculating here |
||
u = dtypes.PeriodPseudoDtype.__new__(cls, -10_000) | ||
u._freq = None | ||
return u | ||
|
||
|
@@ -906,11 +907,15 @@ def __new__(cls, freq=None): | |
try: | ||
return cls._cache[freq.freqstr] | ||
except KeyError: | ||
u = object.__new__(cls) | ||
dtype_code = freq._period_dtype_code | ||
u = dtypes.PeriodPseudoDtype.__new__(cls, dtype_code) | ||
u._freq = freq | ||
cls._cache[freq.freqstr] = u | ||
return u | ||
|
||
def __reduce__(self): | ||
return type(self), (self.freq,) | ||
|
||
@property | ||
def freq(self): | ||
""" | ||
|
@@ -977,7 +982,7 @@ def __eq__(self, other: Any) -> bool: | |
return isinstance(other, PeriodDtype) and self.freq == other.freq | ||
|
||
def __setstate__(self, state): | ||
# for pickle compat. __get_state__ is defined in the | ||
# for pickle compat. __getstate__ is defined in the | ||
# PandasExtensionDtype superclass and uses the public properties to | ||
# pickle -> need to set the settable private ones here (see GH26067) | ||
self._freq = state["freq"] | ||
|
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.
not a huge fan of PeriodPseuodDtype name
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.
im open to suggestions. joris objected to calling it PeriodDtype
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.
this is basically a cython PeriodBaseDtype
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.
Updated with new name + green