Add date overflow message to tz_localize (#32967) #35187
Conversation
pandas/_libs/tslibs/conversion.pyx
Outdated
| @@ -605,13 +605,20 @@ cdef inline check_overflows(_TSObject obj): | |||
| OutOfBoundsDatetime | |||
| """ | |||
| # GH#12677 | |||
| fmt = (f'{obj.dts.year}-{obj.dts.month:02d}-{obj.dts.day:02d} ' | |||
| f'{obj.dts.hour:02d}:{obj.dts.min:02d}:{obj.dts.sec:02d}') | |||
There was a problem hiding this comment.
can you delay evaluating this until we know we're raising?
also pls use double-quotes
There was a problem hiding this comment.
you can use a lambda x: ... here and use .format(obj)
There was a problem hiding this comment.
i think we can milk a tiny bit more performance by avoiding defining the lambda
There was a problem hiding this comment.
well, you end up having to actually either define everything twice (gross) or use a function call, which is precisely what i am suggesting
There was a problem hiding this comment.
define everything twice (gross) or use a function call
Even if you're not evaluating the lambda, defining it at runtime is overhead we can avoid.
i expect the performance difference is tiny, but fstrings showed up in a perf regression @TomAugspurger fixed the other day.
There was a problem hiding this comment.
I just went the defining twice route for now, but I agree it kinda feels gross. Let me know if I should change it.
pandas/_libs/tslibs/conversion.pyx
Outdated
| @@ -605,13 +605,20 @@ cdef inline check_overflows(_TSObject obj): | |||
| OutOfBoundsDatetime | |||
| """ | |||
| # GH#12677 | |||
| fmt = (f'{obj.dts.year}-{obj.dts.month:02d}-{obj.dts.day:02d} ' | |||
| f'{obj.dts.hour:02d}:{obj.dts.min:02d}:{obj.dts.sec:02d}') | |||
There was a problem hiding this comment.
you can use a lambda x: ... here and use .format(obj)
jreback
added
Error Reporting
Hmmm, the only test I found that was actually raising this purposefully was |
|
thanks @gabrielNT |
…ev#35187) * Add date overflow message to tz_localize (pandas-dev#32967) * Delay evaluating timestamp
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffAdded an error message to tz_localize when the timestamp overflows. I got a little confused by the history in #32979 as some changes were lost on the force pushes, but I tried to handle all review comments.