Skip to content

perf: Prefer datetime.fromisoformat over dateutil.parse in Python 3.11+ #657

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

Merged

Conversation

n-thumann
Copy link
Contributor

@n-thumann n-thumann commented May 18, 2024

Proposed Changes

This PR changes the behavior of get_date_helper to prefer datetime.datetime.fromisoformat over dateutil.parser in Python 3.11 or higher.
According to https://github.com/closeio/ciso8601/blob/b17984f5b4c6db0ef596b4d31456c8a39ae278ee/why_ciso8601.md, Since Python 3.11+, the performance of cPython's datetime.fromisoformat is now very good. I did some quick testing and it seems to be on a par with ciso8601:
On Python 3.10 I've got ~37,5s and ~3,5s without and with ciso8601 installed, respectively. On Python 3.11 I've got ~2,5s and ~2,6s without and with ciso8601 installed, respectively.

Because ciso is no dependency, but an extra, not all users (including me) installed it. With this PR these users will get a better out-of-the-box performance for Python 3.11+ users.

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • pytest tests completes successfully
  • Commit messages are conventional
  • Sign CLA (if not already signed)

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.40%. Comparing base (d2393e0) to head (b97b611).
Report is 9 commits behind head on master.

Files Patch % Lines
influxdb_client/client/util/date_utils.py 75.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
- Coverage   90.42%   90.40%   -0.03%     
==========================================
  Files          39       39              
  Lines        3510     3513       +3     
==========================================
+ Hits         3174     3176       +2     
- Misses        336      337       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@n-thumann n-thumann force-pushed the prefer_datetime_over_dateutil_python_3_11 branch from 91eeb51 to 9232a81 Compare May 18, 2024 23:16
@n-thumann n-thumann changed the title Improve performance in Python 3.11+ without ciso6801 extra perf: prefer datetime.fromisoformat over dateutil.parse in Python 3.11+ May 18, 2024
@n-thumann n-thumann changed the title perf: prefer datetime.fromisoformat over dateutil.parse in Python 3.11+ perf: Prefer datetime.fromisoformat over dateutil.parse in Python 3.11+ May 18, 2024
@n-thumann n-thumann force-pushed the prefer_datetime_over_dateutil_python_3_11 branch from 9232a81 to 46f4f7e Compare May 18, 2024 23:21
@bednar bednar added this to the 1.44.0 milestone May 20, 2024
@bednar bednar self-assigned this May 20, 2024
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@n-thumann, thank you for your fantastic PR! 👍

LGTM 🚀

@bednar bednar merged commit 73849e7 into influxdata:master May 20, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants