BUG: empty strings raise in non-ISO8601 formats but parse as NaT elsewhere #50252
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcoGorelli
force-pushed
the
empty-strings
branch
from
479741e to
716d32b
Compare
mroeschke
reviewed
Dec 14, 2022
| def test_empty_string_datetime_coerce_format(): | ||
| # GH13044 | ||
| @pytest.mark.parametrize("errors", ["raise", "coerce", "ignore"]) | ||
| def test_empty_string_datetime_coerce_format(errors): |
Member
There was a problem hiding this comment.
Nit: Maybe good to update this test name since coerce isn't the only errors argument being tested here
mroeschke
reviewed
Dec 14, 2022
Member
mroeschke
left a comment
mroeschke
left a comment
There was a problem hiding this comment.
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
Member
Author
|
good point, thanks - extra test added! |
jbrockmendel
approved these changes
Dec 15, 2022
phofl
approved these changes
Dec 16, 2022
Member
|
thx @MarcoGorelli |
phofl
pushed a commit
to phofl/pandas
that referenced
this pull request
Dec 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.rstfile 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