-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
DEPR: remove freq attribute of Timestamp? #15146
Comments
Is this something we actually want? (if it was possible) |
regular
irregular
So I'll hazard a guess here. I think the reason that we retain the freq attribute is to annotate that we came from a regular frequency (but are representing a timestamp, and not a duration). I don't think it is actually used aywhere. If this is the case, then we should simply remove this; a single timestamp doesn't have a notion of a freq, and if you have multiple timestamps then you have a DTI (which may or may not have a regular freq). I don't think this changes resampling at all (and the decisions of whether to return a PeriodIndex are othogonal here). There may have been another reason, IOW, if you have a Timestamp with a freq, then constructing a NEW DTI doesn't require a freq in theory (though this is not actually implemented). |
cc @wesm if you can shed any light |
Agree with @jreback last comment. Stepping back on the concepts at work here: Frequency for Timestamps are just a helpful label - they don't change the actual value. i.e. I have a temperature reading every day at 7am for 2016: if we have the Timestamp values, 'every day' is just a helpful label / comment, there is no additional information there (and it makes no sense to have that for a single timestamp, then it's just 'a temperature at 7am once') Frequency for Periods is core to its value - it's the length of the interval - i.e. each day in December 2016 I wrote x lines of code: I measure the output between midnight one day and midnight the next day. Because we generally use contiguous indexes, it also is used as the frequency of those intervals. (It could be theoretically be possible to have a PeriodIndex which is Monday every week, for example) |
Is removing the Timestamp.freq attribute even viable at this point? It would make life easier, but seems like a non-trivial break. |
This seems pretty non-actionable at the moment. Might be viable on a 2.0 wishlist? If such a thing existed, there are a handful of tslibs items I'd like to put on it. |
Why would it actually be a wish list item? As described above by others, I changed the title as I don't think this has still something to do with Periods? |
I think simply mentionning the existence of Timestamp with freq value in
the doc would be enough. It was just confusing for me when i stumbled upon
it, but when explained it's just fine and perfectly harmless.
…On 10 Dec 2017 13:17, "Joris Van den Bossche" ***@***.***> wrote:
Why would it actually be a wish list item?
As described above by others, freq on a Timestamp is a *informative label*
at most, but doesn't change the value. I agree with that.
But does it complicate the implementation a lot? I mean, is there a good
reason to get rid of it? (I am not that familiar with that part of the code
base, so it certainly can be it would be nice to get rid of)
I changed the title as I don't think this has still something to do with
Periods?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15146 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AS26JkEmk1IU8QRFSmDnhwcqB7_ukS0Rks5s-9oGgaJpZM4Llu1a>
.
|
The priority is in the range of "well, if we were doing it from scratch, I'd want to do it differently". I figure it's not actionable now, but might be appropriate for 2.0.
In a few small ways. None of these are big deals, but are sufficiently sub-optimal that I can offer the OP moral support.
The only behavior that is affected is |
Now that we have deprecated Timestamp +/- integer ops, this might be worth revisiting. |
If the policy is that anything we want removed in 1.0 needs to be deprecated in 0.24.0, then I think we should do this now. xref #24060 |
Does this have implications for the future of |
No. |
Is this the place to mention use cases where the |
@rwijtvliet you can open an issue but this has already been depreciated |
xref #15141 (comment)
In theory these should be completely fungible. If so, we should remove the
freq
attributeof
Timestamps
and instead returnPeriods
in those instances.The text was updated successfully, but these errors were encountered: