-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DISC: Remove DTA/TDA/PA freq
#24566
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
Comments
I'd probably support this, but would need to think things through a bit more :) The different meaning of freq between period and timedelta / datetime is quite confusing. Additionally
Thinking more though, those apply equally to the index classes, and I suppose that freq would not be deprecated there (nor would we want to?). |
The idea would be that Some use cases of |
In theory, I suppose it could be cached on DatetimeArray/TimedeltaArray as well? I believe that |
Thinking about this again following 1.0 deprecation policy discussion. I'm considering this in conjunction with removing Timestamp.freq (#15146). The difficult part of the implementation for both DTA and Timestamp is that without One option would be to make them into methods to which you can pass a |
I did something vaguely similar with Similarly we can deprecate |
@jbrockmendel your proposal here is to remove it from Datetime/TimedeltaArray, but keep it for the Index classes, right? So dealing with |
Correct. |
And what do you see as the advantage of dealing with this on the Index level instead of the array level? The places where freq gets used (eg resample, plotting?) also support datetimes in columns, not only in the index. For those, it would be beneficial if it was handled on the array-level? |
|
Can you describe that issue in a bit more detail? (or give an example) How is setitem related to views? |
|
Thanks for that example! That clarifies ;) That's an interesting problem. So Index circumvents this by being "immutable". I am wondering if that doesn't point to a general issue with EAs and views. Do we have other EAs with attributes / metadata? We have been thinking about having a faster |
To put it another way: 1) would it technically be possible to implement something similar to "cache_readonly" for EAs (that can handle those problems) and 2) if possible, would it be useful? |
So something that, when cleared/invalidated, would also clear the cache on any other EAs that are a view on the same data? |
I've been experimenting on this, am finding that it is a lot of churn. Having second thoughts about whether this is worth it. A DTA/TDA inside a Series/DataFrame has its freq set to none, so there isn't a real risk there. The only real risk is if a user is dealing with DTA/TDA directly, which is very rare. |
For PA, freq is redundant with dtype. For DTA/TDA, it means something very different from what it means for PA. The only thing it is really used for is _time_shift (and along with it, add/sub of integers, which is deprecated in DTI/TDI anyway)
Without
freq
going into the constructors, DTA/TDA/PA would all just needdtype
, so some simplification/unification could be done.frequency validation in
__init__
could be saved for DTI/TDI and omitted from DTA/TDA.If we ever come to our senses and allow 2D EAs (so we can simplify a ton of internals), freq stops making sense for 2D, so would be unused in those cases anyway.
The text was updated successfully, but these errors were encountered: