-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat(python,rust): add date pattern dd.mm.YYYY
#16045
feat(python,rust): add date pattern dd.mm.YYYY
#16045
Conversation
CodSpeed Performance ReportMerging #16045 will improve performances by 27.87%Comparing Summary
Benchmarks breakdown
|
I think the patterns are assessed in priority order, and I doubt this is the most common form (I'd expect that to be the one separated by "-") 🤔 Can you check? (I'm on mobile at the moment, so can't confirm). |
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.
thanks for your PR
if we go ahead with this, it should also be added to DATETIME_D_M_Y
too (DATE_D_M_Y
should be a subset of it)
@alexander-beedie
@MarcoGorelli |
IIRC it goes in order until it finds a match, then it keeps using the last successful one (until it may need to look at the next one in the group). maybe let's put them in order of popularity then?
ok sure |
sure! But how do we measure this?
No sure what the performance impact really is but if we do this "micro optimizations" should we also adjust the |
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.
thanks @JulianCologne - probably not worth micro-optimising the order, and as you noted, %d.%m.%Y
is very common
close #14990
dd.mm.YYYY
date pattern which is widely used.to_date
as well as inread_csv