Skip to content

CI: avoid guess_datetime_format failure on 29th of Feburary #57674

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 5, 2024
Merged

CI: avoid guess_datetime_format failure on 29th of Feburary #57674

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bdb6d8
try fix ci
MarcoGorelli Feb 29, 2024
d7eab7c
add comment
MarcoGorelli Feb 29, 2024
a520cfe
Merge remote-tracking branch 'upstream/main' into fix-ci
MarcoGorelli Mar 1, 2024
d9c330b
Merge branch 'main' into fix-ci
MarcoGorelli Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions pandas/_libs/tslibs/parsing.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,8 +936,10 @@ def guess_datetime_format(dt_str: str, bint dayfirst=False) -> str | None:
datetime_attrs_to_format.remove(day_attribute_and_format)
datetime_attrs_to_format.insert(0, day_attribute_and_format)

# same default used by dateutil
default = datetime.now().replace(hour=0, minute=0, second=0, microsecond=0)
# Use this instead of the dateutil default of
# `datetime.now().replace(hour=0, minute=0, second=0, microsecond=0)`
# as that causes issues on the 29th of February.
default = datetime(1970, 1, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to understand what's going on. Surely happy to get this merged if it fixes the problem and you understand what's going on.

The root problem seems to be that guess_datetime_format('2003') is None instead of %Y. That would be easy to test if we want. What I don't understand is why changing the day and the month in the parsed date is relevant if the only token we have is the year...

>>> dateutil.parser.parse('2003', dayfirst=True, default=datetime.datetime.now().replace(hour=0, minute=0, second=0, microsecond=0))
datetime.datetime(2003, 2, 28, 0, 0)
>>> dateutil.parser.parse('2003', dayfirst=True, default=datetime.datetime(1970, 1, 1))
datetime.datetime(2003, 1, 1, 0, 0)

I didn't debug it, I guess you did, but I can't see why in the implementation it should make a difference. As said, if you are confident that this is a good solution and the algorithm is relivable happy to get this merged. But seems like there is a bug in the algorithm below we are not preventing/hacking, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah seems like there's something in pandas' vendored code which makes dateutil_parse fail, even though non-vendored dateutil itself parses it. that's the problem with vendoring I guess...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datapythonista - currently in main we take today's date, replace it with the bits that the user passed, and then check if it's a valid date. E.g. "2002" on 2/29 becomes "2002-02-29" which is not a valid date. From this we conclude we've somehow guessed wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks both. I see, I assumed dateutil_parse was an alias to the dateutil parser. I see the copied version has the comment lifted from dateutil to get resolution, maybe the dateutil parser works at second resolution and we work at microsecond or nanosecond, and that's why the function was copied to pandas?

In any case I think the fix is fine, a function to get the format from a partial date is surely not going to be perfect anyway.

try:
parsed_datetime = dateutil_parse(
dt_str,
Expand Down