-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ERR: "day out of range" doesn't show position of error #50464
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
af31616
to
83274a0
Compare
83274a0
to
b283338
Compare
raise ValueError(f"time data \"{val}\" at position {i} doesn't " | ||
raise ValueError(f"time data \"{val}\" doesn't " |
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.
Rather than trying to set as position {i}
in each place where an error can occur, it's more reliable to just preprend at position {i}
when an error is caught (see below)
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.
I haven't done the same in tslib.pyx because when format
is provided, that code is going to be called here anyway in #50242
So for now I've just made sure the error messages match for the two paths
b283338
to
368e38a
Compare
( | ||
"2015-04-31", | ||
"%Y-%m-%d", | ||
'^time data "2015-04-31" doesn\'t match format "%Y-%m-%d", ' | ||
"at position 0$", | ||
), | ||
( | ||
"2015-31-04", | ||
"%Y-%d-%m", | ||
"^day is out of range for month, at position 0$", | ||
), |
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.
note how the ISO and non-ISO messages don't quite match yet - this PR just ensures that they both show the position of the error
#50242 will ensure that the messages match
Thanks @MarcoGorelli |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Haven't added a whatsnew note, as this can be considered a follow-up to #50366