Skip to content

BUG: Add AM/PM token support on guess_datetime_format #53257

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

Jump to bottom
Merged
merged 11 commits into from
May 17, 2023
Merged

BUG: Add AM/PM token support on guess_datetime_format #53257

merged 11 commits into from
May 17, 2023

Conversation

juli4nb4dillo
Copy link
Contributor

mroeschke
mroeschke reviewed May 16, 2023
View reviewed changes
doc/source/whatsnew/v2.1.0.rst Outdated
@@ -352,7 +352,7 @@ Conversion
- Bug in :meth:`DataFrame.__repr__` incorrectly raising a ``TypeError`` when the dtype of a column is ``np.record`` (:issue:`48526`)
- Bug in :meth:`DataFrame.info` raising ``ValueError`` when ``use_numba`` is set (:issue:`51922`)
- Bug in :meth:`DataFrame.insert` raising ``TypeError`` if ``loc`` is ``np.int64`` (:issue:`53193`)
-
- Bug in :meth:`datetimes._guess_datetime_format` if contains "AM" / "PM" tokens (:issue:`53147`)
Copy link
Member

Choose a reason for hiding this comment

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

Could you rewrite this in terms of :func:`to_datetime` being affected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, please check.

@mroeschke mroeschke added the Datetime Datetime data dtype label May 16, 2023
@mroeschke mroeschke mentioned this pull request May 16, 2023
Closed
5 tasks
@juli4nb4dillo
Copy link
Contributor Author

@MarcoGorelli I tried it from the top, it was a dummy bug, but I almost couldn't figure it out until I setup my local env.

@MarcoGorelli MarcoGorelli self-requested a review May 16, 2023 19:02
MarcoGorelli
MarcoGorelli approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

wonderful, nice work!

@mroeschke wondering if this should be backported to 2.0.2 actually, as pdep-4 was new in 2.0

EDIT: yeah let's backport, guess_datetime_format only started being used by default in 2.0 so I'd consider this a regression. Will merge later today if there's no other comments

@MarcoGorelli MarcoGorelli added this to the 2.0.2 milestone May 17, 2023
@mroeschke mroeschke merged commit f2de598 into pandas-dev:main May 17, 2023
@mroeschke
Copy link
Member

Thanks @juli4nb4dillo

@lumberbot-app
Copy link

lumberbot-app bot commented May 17, 2023

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 2.0.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 f2de598d9002801e2056b4f8e2f587fd8bb07bc1
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #53257: BUG: Add AM/PM token support on guess_datetime_format'
  1. Push to a named branch:
git push YOURFORK 2.0.x:auto-backport-of-pr-53257-on-2.0.x
  1. Create a PR against branch 2.0.x, I would have named this PR:

"Backport PR #53257 on branch 2.0.x (BUG: Add AM/PM token support on guess_datetime_format)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

MarcoGorelli added a commit to MarcoGorelli/pandas that referenced this pull request May 17, 2023
@juli4nb4dillo @MarcoGorelli
'Backport PR pandas-dev#53257: BUG: Add AM/PM token support on guess_…
e456a96 
…datetime_format'

* BUG: Add am/pm parsing support on guess_format

* ⛏️ add unit tests

* more unit tests

* ⛏️ not guess am/pm mark

* 🪲 bug fix parsing am/pm tokens

* 📝 whatsnew

* 📝 whats new doc

* isort on whatsnew entry

* move whatsnew note to 2.0.1

* 2.0.2, not 2.0.1

* clarify

---------

Co-authored-by: MarcoGorelli <[email protected]>
(cherry picked from commit f2de598)
MarcoGorelli added a commit to MarcoGorelli/pandas that referenced this pull request May 17, 2023
@juli4nb4dillo @MarcoGorelli
Backport PR pandas-dev#53257: BUG: Add AM/PM token support on guess_d…
4232d53 
…atetime_format

---------

Co-authored-by: MarcoGorelli <[email protected]>
(cherry picked from commit f2de598)
mroeschke pushed a commit that referenced this pull request May 17, 2023
@MarcoGorelli @juli4nb4dillo
"Backport PR #53257 on branch 2.0.x (BUG: Add AM/PM token support on …
456a041 
…guess_datetime_format)" (#53277)

Backport PR #53257: BUG: Add AM/PM token support on guess_datetime_format

---------

Co-authored-by: MarcoGorelli <[email protected]>
(cherry picked from commit f2de598)

Co-authored-by: Julian Badillo <[email protected]>
topper-123 pushed a commit to topper-123/pandas that referenced this pull request May 22, 2023
@juli4nb4dillo @MarcoGorelli @topper-123
BUG: Add AM/PM token support on guess_datetime_format (pandas-dev#53257)
aaafa0d 
* BUG: Add am/pm parsing support on guess_format

* ⛏️ add unit tests

* more unit tests

* ⛏️ not guess am/pm mark

* 🪲 bug fix parsing am/pm tokens

* 📝 whatsnew

* 📝 whats new doc

* isort on whatsnew entry

* move whatsnew note to 2.0.1

* 2.0.2, not 2.0.1

* clarify

---------

Co-authored-by: MarcoGorelli <[email protected]>
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
@juli4nb4dillo @MarcoGorelli
BUG: Add AM/PM token support on guess_datetime_format (pandas-dev#53257)
82b35da 
* BUG: Add am/pm parsing support on guess_format

* ⛏️ add unit tests

* more unit tests

* ⛏️ not guess am/pm mark

* 🪲 bug fix parsing am/pm tokens

* 📝 whatsnew

* 📝 whats new doc

* isort on whatsnew entry

* move whatsnew note to 2.0.1

* 2.0.2, not 2.0.1

* clarify

---------

Co-authored-by: MarcoGorelli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

@MarcoGorelli MarcoGorelli MarcoGorelli approved these changes

Assignees
No one assigned
Labels
Datetime Datetime data dtype
Projects
None yet
Milestone
2.0.2
Development

Successfully merging this pull request may close these issues.

BUG: to_datetime ignores "AM" and "PM" unless format is explicitly set
3 participants
@juli4nb4dillo @mroeschke @MarcoGorelli