AndrewAnnex / SpiceyPy

SpiceyPy: a Pythonic Wrapper for the SPICE Toolkit.
MIT License
398 stars 84 forks source link

use datetime.fromisoformat instead of strptime #384

Closed johan12345 closed 4 years ago

johan12345 commented 4 years ago

to improve performance of et2datetime.

The performance of the datetime parsing itself is enhanced by a factor of 100:

timeit.timeit('datetime.datetime.strptime("2020-09-09T15:00:00.000", "%Y-%m-%dT%H:%M:%S.%f").replace(tzinfo=datetime.timezone.utc)', setup='import datetime')
>>> 10.533629100000013
timeit.timeit('datetime.datetime.fromisoformat("2020-09-09T15:00:00.000").replace(tzinfo=datetime.timezone.utc)', setup='import datetime')
>>> 1.2155922999999973
timeit.timeit('datetime.datetime.fromisoformat("2020-09-09T15:00:00.000+00:00")', setup='import datetime')
>>> 0.16864590000000135

This leads to a total performance improvement of ~60% for et2datetime:

# before
timeit.timeit('spice.et2datetime(200)', setup='import spiceypy as spice; spice.furnsh("latest_leapseconds.tls")')
>>> 27.4600037
# after
timeit.timeit('spice.et2datetime(200)', setup='import spiceypy as spice; spice.furnsh("latest_leapseconds.tls")')
>>> 11.986385900000002

datetime.fromisoformat is only available since Python 3.7, I added a fallback to strptime for Python 3.6.

pep8speaks commented 4 years ago

Hello @johan12345! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Comment last updated at 2020-09-09 14:41:23 UTC
AndrewAnnex commented 4 years ago

hey @johan12345, thanks for the contribution! I see that pep8 has a problem with using lambdas, once the pr has been updated to use defs instead it will be good to go.

johan12345 commented 4 years ago

Sure! I have replaced the lambdas with defs now.

codecov[bot] commented 4 years ago

Codecov Report

Merging #384 into main will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #384   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files          12       12           
  Lines       15106    15110    +4     
=======================================
+ Hits        15088    15092    +4     
  Misses         18       18           
Impacted Files Coverage Δ
spiceypy/spiceypy.py 99.69% <100.00%> (+<0.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9aa3ade...9816306. Read the comment docs.