Raise NotImplementedError for datetime strings with UTC offset #14070
Conversation
mroeschke
added
bug
| try: | ||
| pd_arbitrary = pd.to_datetime(arbitrary) | ||
| except pd.errors.OutOfBoundsDatetime: | ||
| # Not an issue in pandas>=2.0 |
There was a problem hiding this comment.
Does this comment mean that this error won't be raised when we upgrade to pandas 2.0? If so, one suggestion I would have is that we implement a utility function that will raise an exception everywhere that we have a pandas<2.0 code path if the detected pandas version is 2.0. Then when we actually upgrade pandas we can validate that we changed everywhere that needed changing.
There was a problem hiding this comment.
Ah so actually this doesn't work yet in pandas (it was related to non-nanosecond support in pandas 2.0, but it's not implemented everywhere yet). I linked the issue to this exception path
| @@ -2150,6 +2150,12 @@ def test_daterange_pandas_compatibility(): | |||
| assert_eq(expected, actual) | |||
|
|
|||
|
|
|||
| @pytest.mark.filterwarnings("ignore:parsing timezone:DeprecationWarning") | |||
There was a problem hiding this comment.
Can we use pytest.warns here instead? See https://docs.rapids.ai/api/cudf/stable/developer_guide/testing/#testing-code-that-throws-warnings.
There was a problem hiding this comment.
Sure thing, added
| except pd.errors.OutOfBoundsDatetime: | ||
| # https://github.com/pandas-dev/pandas/issues/55096 | ||
| pass | ||
| else: |
There was a problem hiding this comment.
nitpick (non-blocking): style
FWIW I tend to prefer not using else on try/except (I can never remember if it applies to the try or the except), so perhaps move this validation into the try block?
There was a problem hiding this comment.
Sure thing. Moved to the try block
| @@ -2150,6 +2150,12 @@ def test_daterange_pandas_compatibility(): | |||
| assert_eq(expected, actual) | |||
|
|
|||
|
|
|||
| def test_strings_with_utc_offset_not_implemented(): | |||
| with pytest.warns(DeprecationWarning, match="parsing timezone"): | |||
| with pytest.raises(NotImplementedError): | |||
| @@ -2150,6 +2150,12 @@ def test_daterange_pandas_compatibility(): | |||
| assert_eq(expected, actual) | |||
|
|
|||
|
|
|||
| def test_strings_with_utc_offset_not_implemented(): | |||
| with pytest.warns(DeprecationWarning, match="parsing timezone"): | |||
There was a problem hiding this comment.
And this one is pandas in the pd.to_datetime call?
There was a problem hiding this comment.
This one is in cupy. I'll add a comment referencing this
|
/merge |
Description
Avoids e.g. DatetimeIndex(["2022-07-22 00:00:00+02:00"]) from dropping the +02:00 since timezones are not supported
Checklist