BUG: when flooring, ambiguous parameter unnecessarily used (and raising Error)

[rwijtvliet]

Pandas version checks

• I have checked that this issue has not already been reported.

• 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

INSTALLED VERSIONS

commit : 91111fd
python : 3.9.13.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.19045
machine : AMD64
processor : Intel64 Family 6 Model 165 Stepping 2, GenuineIntel
byteorder : little
LC_ALL : None
LANG : None
LOCALE : de_DE.cp1252

pandas : 1.5.1
numpy : 1.23.4
pytz : 2022.6
dateutil : 2.8.2
setuptools : 63.4.1
pip : 22.2.2
Cython : None
pytest : 7.1.3
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : None
IPython : 8.5.0
pandas_datareader: None
bs4 : None
bottleneck : None
brotli : None
fastparquet : None
fsspec : None
gcsfs : None
matplotlib : 3.6.2
numba : None
numexpr : None
odfpy : None
openpyxl : 3.0.10
pandas_gbq : None
pyarrow : None
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : None
snappy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
zstandard : None
tzdata : None

[jcaoun]

I can take this one

[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.

[MarcoGorelli]

Agree with @mroeschke , it's fine to error and the error message is clear - closing for now then

[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.

[MarcoGorelli]

thanks - reopening for now, will take a closer look later

[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?

[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') --> AmbiguousTimeError
• ts_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.

[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.floor

I 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

[MarcoGorelli]

OK let's reopen, I'll leave some comments there

[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 #37592 (comment)