-
-
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
API: Make 'D' & offsets.Day always operate as calendar day instead of 24 Hour #22864
Comments
Didn't look yet into the WIP PR, but some thoughts:
|
So to actually better answer to your top post .. Regarding deprecation: I think we need a deprecation for each of the 4 points you mention. Eg case 1 (Day arithmetic with Timestamp) will break people's code if we simply change it. Of course, it will only change the result in a few cases (only when you have tz-ware data, and when there is a DST), so we might think about only raising a warning then. |
Just so I understand @jorisvandenbossche:
So in my PR, I can just prep the new |
Yes, that is how I would do it (but we can still discuss of course). |
We can discuss this during the next dev chat, but I want to confirm the operations that warrant Timedelta Related Deprecations
Datetime Related Deprecations
Tick Related Deprecations
I've already tried to implement some of these warnings (like warning for |
Conversely, if we keep the current behavior in master. We would need to add
|
Post dev discussion on 11/8/2018. My understanding of the conclusion:
Question here:
Given the above example, it may make sense to have both a |
If we're talking reasonably long-term, I think we should get rid of the |
I'd be on board with that, but I suspect it might be a lot of effort? Just so I'm on the same page, would |
Moving this off the 0.24.0 milestone, this can be address in a future release. #24330 just removes the existing |
@mroeschke ive totally lost track, where did we land on this? |
I think we wanted @jbrockmendel you mentioned:
which would make this transition a bit easier I think. |
@mroeschke this came up on the call today. any chance youll have time for this in the not-too-distant future? |
Sure I can start chipping away at this again. I remember propagating all the warnings correctly will be the biggest challenge. |
Closing in favor of #41943 |
From the dev-chat #22274 (comment) and a rehash of #20633
Conclusion:
'D'
andoffsets.Day
always operate as a calendar day.Technical implication:
Day
will need to subclassDateOffset
instead ofTick
Operations that will change behavior (may be missing some):
Day
arithmetic with Timestamp/DTI/datetime Series/DataFrameDatetimeIndex.shift
'D'
with Timedelta/TDI/timedelta Series/DataFrameDay
Deprecation Procedures (could use some input here)
DeprecationWarning
that users should use'24 H'
instead of'D'
and allow users to use'D'
for v0.24.0cc @pandas-dev/pandas-core
The text was updated successfully, but these errors were encountered: