Pandas version checks
-
[X] I have checked that this issue has not already been reported.
-
[X] I have confirmed this bug exists on the latest version of pandas.
-
[ ] I have confirmed this bug exists on the main branch of pandas.
Reproducible Example
import pandas as pd
ts = pd.Timestamp("2020-10-25 00:00", tz='Europe/Berlin')
# This works:
ts.floor('H') # Timestamp('2020-10-25 00:00:00+0200', tz='Europe/Berlin')
# But this gives an error:
ts.floor('H', ambiguous='infer') # ValueError: Cannot infer offset with only one time.
Issue Description
The value of the ambiguous parameter is used by the method, even if there is no ambiguity.
Expected Behavior
Both calls to the .floor method should give the same outcome.
Installed Versions
Comment From: jcaoun
I can take this one
Comment From: mroeschke
I believe the explicitness of the passing ambiguous= to Timestamp.floor is a better behavior than silently ignoring it (though the floor can be valid) so I would deem this the expected behavior.
Comment From: MarcoGorelli
Agree with @mroeschke , it's fine to error and the error message is clear - closing for now then
Comment From: rwijtvliet
I've dug some deeper, and I'm confused.
On the one hand, according to the docs, using ambiguous='infer' in Timestamp.floor() is not allowed:
ambiguous : bool or {'raise', 'NaT'}, default 'raise'
The behavior is as follows:
* bool contains flags to determine if time is dst or not (note
that this flag is only applicable for ambiguous fall dst dates).
* 'NaT' will return NaT for an ambiguous time.
* 'raise' will raise an AmbiguousTimeError for an ambiguous time.
(I don't understand thebool contains flags part, but let's ignore it here.)
Going off that, my usage is incorrect and this ticket can be closed.
On the other hand, it seems that the documentation is not always correct. Relevant here: in the behavior of Timestamp.floor(), 'infer' is actually treated separately, as witnessed from the error message (ValueError: Cannot infer offset with only one time.).
I still think the current behavior is not ideal, having to wrap each call in a try...except block to try both with and without ambiguous. It's also not consistent with how the ambiguous parameter is used in other methods, such as DatetimeIndex.tz_localize():
ambiguous : 'infer', 'NaT', bool array, default 'raise'
When clocks moved backward due to DST, ambiguous times may arise.
For example in Central European Time (UTC+01), when going from
03:00 DST to 02:00 non-DST, 02:30:00 local time occurs both at
00:30:00 UTC and at 01:30:00 UTC. In such a situation, the
`ambiguous` parameter dictates how ambiguous times should be
handled.
Note the in such a situation. In other words, the value of the ambiguous parameter is only used if things are, well, ambiguous. In my opinion, this should also be the case in Timestamp.floor.
Comment From: MarcoGorelli
thanks - reopening for now, will take a closer look later
Comment From: MarcoGorelli
Ah good spot, thanks for raising this
Going off that, my usage is incorrect and this ticket can be closed.
I don't think your usage is incorrect - rather, it would probably be better if pandas were to raise if anything other than a boolean, 'raise', or 'NaT' is passed to Timestamp.floor
I still think the current behavior is not ideal, having to wrap each call in a try...except block to try both with and without ambiguous.
not sure what you mean here, sorry - could you expand please?
Comment From: rwijtvliet
Thanks for having another look. Here's my comment, sorry it's a bit long.
My last comment is no longer valid the way I initially meant it. In the original example, all valid values of ambiguous (None, True, False, 'raise', 'NaT') do return the correct value in ts.floor('H', ...), namely 2020-10-25 00:00:00+02:00, so no error catching is needed there. The .floor() method ignores the value of ambiguous, which is what I'd expect, as there is no ambiguity.
However, I also found another unambiguous timestamp, where the value of ambiguous is required (and not ignored), and where error catching is needed. And that's the repeated hour at the end of DST. pandas is clever enough when constructing the timestamp, with an error being correctly raised if things are ambiguous. We can make things unambiguous by including the UTC offset:
ts_ambiguous = pd.Timestamp("2020-10-25 02:30", tz='Europe/Berlin') #AmbiguousTimeError
ts_unamb = pd.Timestamp("2020-10-25 02:30+0200", tz='Europe/Berlin') # no error, because not ambiguous
So far, so good. As it's clear from the UTC offset that ts_unamb falls between 02:00+0200 and 02:00+0100, flooring should be possible, and ts_unamb.floor('H') should return pd.Timestamp("2020-10-25 02:00+0200", tz='Europe/Berlin'). Yet, this is not what happens. What happens is:
ts_unamb.floor('H')-->AmbiguousTimeErrorts_unamb.floor('H', ambiguous=False)-->Timestamp('2020-10-25 02:00:00+0100', tz='Europe/Berlin')(incorrect)ts_unamb.floor('H', ambiguous=True)-->Timestamp('2020-10-25 02:00:00+0200', tz='Europe/Berlin')(the correct answer)
This is where it's then necessary to catch errors, e.g. like so:
import pytz
import pandas as pd
def floor(ts: pd.Timestamp) -> pd.Timestamp:
"""Floor ``ts`` to nearest hour, assuming the Europe/Berlin timezone."""
try:
return ts.floor('H')
except pytz.AmbiguousTimeError:
if ts.utcoffset().total_seconds() == 7200:
return ts.floor('H', ambiguous=True)
else:
return ts.floor('H', ambiguous=False)
floor(pd.Timestamp('2020-10-25 02:30+0200', tz='Europe/Berlin')) # Timestamp('2020-10-25 02:00:00+0200', tz='Europe/Berlin')
floor(pd.Timestamp('2020-10-25 02:30+0100', tz='Europe/Berlin')) # Timestamp('2020-10-25 02:00:00+0100', tz='Europe/Berlin')
This error catching probably leaves out edge cases and might return incorrect results in cases where ts actually is ambiguous; it'd be an improvement if pandas would only raise an AmbiguousTimeError in case of an actual ambiguity.
Comment From: jcaoun
I don't think your usage is incorrect - rather, it would probably be better if pandas were to raise if anything other than a boolean, 'raise', or 'NaT' is passed to
Timestamp.floorI believe the code I pushed before fixes this issue. Before I pushed my code, when I tested Timestamp.floor with something like this: ts.floor('H', ambiguous='hi') , it didn't throw an error and just returned the timestamp as normal (although this was on my local machine so someone should verify this is the actual behavior). Either way, I think that either all invalid arguments to ambiguous should be treated the same, or we add support for 'infer' in Timestamp.floor
Comment From: MarcoGorelli
OK let's reopen, I'll leave some comments there
Comment From: mroeschke
As it's clear from the UTC offset that ts_unamb falls between 02:00+0200 and 02:00+0100, flooring should be possible, and ts_unamb.floor('H') should return pd.Timestamp("2020-10-25 02:00+0200", tz='Europe/Berlin')
Well since floor/ceil/round is done on the wall time (i.e. timestamp.tz_localize(None).floor(freq).tz_localize(timestamp.tz)) it's not possible to "know" that +0200 is necessarily the "correct" offset. But ideally I suppose the "original offset" should be considered https://github.com/pandas-dev/pandas/issues/37592#issuecomment-721502737