REF: mix PeriodPseudoDtype into PeriodDtype #34590
Conversation
| @@ -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.
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.
Yes. We have a test that constructs PeriodDtype() which goes through this path. the existing comment a couple lines up suggests this is for pickle compat
There was a problem hiding this comment.
right, how does this get resolved though? do you see periods with undefined after this?
There was a problem hiding this comment.
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
pandas/core/dtypes/dtypes.py
Outdated
| @@ -848,7 +848,7 @@ def __setstate__(self, state) -> None: | |||
|
|
|||
|
|
|||
| @register_extension_dtype | |||
| class PeriodDtype(PandasExtensionDtype): | |||
| class PeriodDtype(dtypes.PeriodPseudoDtype, PandasExtensionDtype): | |||
There was a problem hiding this comment.
not a huge fan of PeriodPseuodDtype name
There was a problem hiding this comment.
im open to suggestions. joris objected to calling it PeriodDtype
There was a problem hiding this comment.
this is basically a cython PeriodBaseDtype
There was a problem hiding this comment.
Updated with new name + green
|
looks good. slightly concerned about the UNDEFINED case (e.g. we don't assert this anywhere)........ |
It also shows up in period.pyx as FR_UND (xref #34588), so i dont think we'll be able to get rid of it, but we should at least be able to get to a point with fewer of these enum-like codes lying around |
This will allow us to start getting rid of
get_freq_codeusages and related libfrequencies functions that are unnecessarily roundabout.