-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: empty strings raise in non-ISO8601 formats but parse as NaT elsewhere #50252
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
479741e
to
716d32b
Compare
def test_empty_string_datetime_coerce_format(): | ||
# GH13044 | ||
@pytest.mark.parametrize("errors", ["raise", "coerce", "ignore"]) | ||
def test_empty_string_datetime_coerce_format(errors): |
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.
Nit: Maybe good to update this test name since coerce
isn't the only errors argument being tested here
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.
One comment otherwise this change looks reasonable.
Also though it might be redundant, might be good to have an independent test showing that iso & non-iso format with empty string produces NaT
good point, thanks - extra test added! |
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.
LGTM
thx @MarcoGorelli |
closes BUG: empty strings raise in non-ISO8601 formats but parse as NaT elsewhere #50251 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.ISO8601 formats make empty strings NaT
non-ISO8601 formats raise
Let's make this consistent. I think I prefer the former (
NaT
). ISO formats are probably also more common, so arguably this'd be a breaking change for fewer people