-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: improve date performance regression #18756
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
FIX: improve date performance regression #18756
Conversation
If I add a timezone,
then its a bit slower: 3.29 s or so, which seems an acceptable slow down. |
... I don't understand the codecov error - this PR didn't touch the tests... |
@QuLogic is it possible to add datetime handling to the performance tests? |
2a5f102
to
d4e5ddc
Compare
tzi = getattr(d[0], 'tzinfo', None) | ||
if tzi is not None: | ||
# make datetime naive: | ||
d = [dt.astimezone(UTC).replace(tzinfo=None) for dt in d] |
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 made this one statement after @tacaswell's approval (sorry). There were two list comprehensions, and I realized it could just as easily be one!
Note, as promised, that there is now a benchmark PR in mpl-benchmark |
…756-on-v3.3.x Backport PR #18756 on branch v3.3.x (FIX: improve date performance regression)
Hi @QuLogic, I'm intrigued by how the meeseeksmachine was utilized for the backporting process in this PR. Can you please shed some light on how it's triggered? Is it through specific labels or comments, or does it run automatically upon merging? |
@Alexia-I here is our documentation on the automatic backports In this case it was triggered by the milestone but it can also be triggered by tagging the bot in a comment. |
@rcomer Thank you! |
PR Summary
Closes #18743
date2num
was slowed down for long lists of datetime. This pr simply tries to convert them to datetime64 before we convert to a num which seems fast.I think that if we had a list of date times with timezones it would still be quite slow, but this fixes the naive-datetime case. Open to better suggestions..
Master
This PR:
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
andpydocstyle<4
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).