-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Add AM/PM token support on guess_datetime_format #53244
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your PR - there's some failing tests, could you fix them up please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks - will need a whatsnew note for 2.1 too
pandas/tests/tslibs/test_parsing.py
Outdated
("2005/11/09 10:15 AM", "%Y/%m/%d %I:%M %p"), | ||
("2005/11/09 10:15:32 AM", "%Y/%m/%d %I:%M:%S %p"), | ||
("Tue 24 Aug 2021 01:30:48 AM", "%a %d %b %Y %H:%M:%S %p"), | ||
("Tuesday 24 Aug 2021 01:30:48 AM", "%A %d %b %Y %H:%M:%S %p"), | ||
("Tue 24 Aug 2021 01:30:48", "%a %d %b %Y %H:%M:%S"), | ||
("Tuesday 24 Aug 2021 01:30:48", "%A %d %b %Y %H:%M:%S"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you keep these in but change the expected fmt
(the second argument) to be None
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let me see if I can setup my env to build and test locally, otherwise I'm kinda flying blind waiting for Github Actions to run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could try gitpod if you're having trouble, else feel free to ask on the slack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm... it guesses the format to be "%a %d %b %Y %H:%M:%S AM"
instead of None
, which has to do with this loop:
pandas/pandas/_libs/tslibs/parsing.pyx
Lines 1012 to 1018 in 8894dcc
try: | |
# If the token is numeric, then we likely didn't parse it | |
# properly, so our guess is wrong | |
float(tokens[i]) | |
return None | |
except ValueError: | |
pass |
Let me try again from the top, I think we can make it work with the "AM/PM" tokens easier now that I can test locally.
Is this PR or #53257 supposed to fix this? |
@mroeschke It's duplicate, sorry for that, I wanted to test it from the top so I made another branch. |
doc/source/whatsnew/v2.1.0.rst
file if fixing a bug or adding a new feature.