Fix for B.Y.d format corner case #687#700
Conversation
|
This more or less looks good to me, though it does seem like a concerning amount of overhead to fix this very specific corner case. Not a big deal though. |
Yah, the more specific fix would have been at L498 to check The change from 501-511 could be considered orthogonal. |
pganssle
left a comment
There was a problem hiding this comment.
Some minor performance concerns, but nothing blocking. l've restructured the code a bit to hopefully improve performance, and I'd say it's ready to merge now. @jbrockmendel Any objections to my changes?
|
Looks fine. Is the perf really affected? If so then constructing Aesthetically I like the |
|
I doubt it will really make a major difference in the long run given that the parser is not amazingly performant to start with, but I'd like to minimize the performance impact since this particular bug is for such an incredibly niche problem. From micro-benchmarks I would estimate I saved a few microseconds by organizing it this way. |
Replaces #698.
At the moment
_ymd.resolve_ymdcan ignoreystridxanddstridxin some corner cases. This PR hasresolve_ymdcheck if enough of[ystridx, mstridx, dstridx]have been specified to uniquely pin down (year, month, day), and if so, use those instead of other heuristics.Edits resolve_ymd to prevent deleting an element in-place, which makes step-through debugging more difficult.
Closes #687
Pull Request Checklist