BUG guess_Datetime_format doesn't guess 27.03.2003 14:55:00.000 #50319
Conversation
pandas/_libs/tslibs/parsing.pyx
Outdated
| if re.search(r"\d*\.\d+", token) is None: | ||
| # For example: 98 | ||
| token_filled = token.zfill(padding) | ||
| else: | ||
| # For example: 00.123 | ||
| seconds, nanoseconds = token.split(".") | ||
| seconds = f"{int(seconds):02d}" |
There was a problem hiding this comment.
previously,
seconds = f"{int(seconds):02d}"
was failing if token was just a single character '.'
If we make the check more precise, then we avoid that issue
mroeschke
left a comment
mroeschke
left a comment
There was a problem hiding this comment.
LGTM. Doesn't need a whatsnew right?
pandas/_libs/tslibs/parsing.pyx
Outdated
| cdef str _fill_token(token: str, padding: int): | ||
| cdef str token_filled | ||
| if "." not in token: | ||
| if re.search(r"\d*\.\d+", token) is None: |
There was a problem hiding this comment.
is the leading \d* needed here?
There was a problem hiding this comment.
\d+ would be better actually, thanks
This is for when it corresponds with the %S.%f directive
thanks! and no, this wouldn't (yet) make a user-facing difference it fixes #2586 by accident, but I'd say that that fits into the PDEP4-related part of the whatsnew (as in, PDEP4 should have fixed that, it just didn't quite because of an error in implementation here) |
mroeschke
left a comment
mroeschke
left a comment
There was a problem hiding this comment.
LGTM. Merge when ready @jbrockmendel
| token_filled = token.zfill(padding) | ||
| else: | ||
| # For example: 00.123 | ||
| seconds, nanoseconds = token.split(".") |
There was a problem hiding this comment.
is there a risk that this raises if there is a "." but not the regex?
There was a problem hiding this comment.
yup, you'd get
In [6]: token = '.'
In [7]: seconds, nanoseconds = token.split(".")
In [8]: seconds = f"{int(seconds):02d}"
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
Cell In [8], line 1
----> 1 seconds = f"{int(seconds):02d}"
ValueError: invalid literal for int() with base 10: ''which is what happens in the string in the linked issue ('27.03.2003 14:55:00.000')
There was a problem hiding this comment.
i take it this is caught elsewhere so isnt a problem?
There was a problem hiding this comment.
yeah, if the padding was 2 (say) then '.'.zfill(2) would give '0.', and in
pandas/pandas/_libs/tslibs/parsing.pyx
Lines 945 to 946 in f7979be
it wouldn't match parsed_formatted
|
merging as there's been an approval, and this is quite minor, and it unblocks another PR - if there's any other concerns / feedback I can address them in a follow-up |
3 participants
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.