Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(targets): Parse dates with datetime.fromisoformat/backports.datetime_fromisoformat #2070

Conversation

BuzzCutNorman
Copy link
Contributor

@BuzzCutNorman BuzzCutNorman commented Nov 27, 2023

Replaced python-dateutil.parse with backports.datetime_fromisoformat.
Changed except to ValueError.

Closes #2046


📚 Documentation preview 📚: https://meltano-sdk--2070.org.readthedocs.build/en/2070/

Copy link

codspeed-hq bot commented Nov 27, 2023

CodSpeed Performance Report

Merging #2070 will improve performances by ×45

Comparing BuzzCutNorman:2046-refactor-parse-dates-with-backports-datetime-fromisoformat (fbeff42) with main (b4a7b3e)

Summary

⚡ 2 improvements
✅ 2 untouched benchmarks

Benchmarks breakdown

Benchmark main BuzzCutNorman:2046-refactor-parse-dates-with-backports-datetime-fromisoformat Change
test_bench_parse_timestamps_in_record 2,513.3 ms 55.7 ms ×45
test_bench_validate_and_parse 2,968.9 ms 512.6 ms ×5.8

Copy link
Collaborator

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

I think the mypy failure can be fixed by adding backports-datetime-fromisoformat here:

https://github.com/buzzcutnorman/sdk/blob/a4bdd6674f5f59fda60fbad9dd01af046676dd6d/noxfile.py#L70

pyproject.toml Outdated Show resolved Hide resolved
@edgarrmondragon edgarrmondragon changed the title refactor: Parse dates with backports.datetime_fromisoformat refactor(targets): Parse dates with backports.datetime_fromisoformat Nov 27, 2023
@BuzzCutNorman
Copy link
Contributor Author

I should have noticed this initially but backports-datetime-fromisoformat is not a datetime parser. It does not look at the input and see if datetime.datetime.fromisoformat(), datetime.date.fromisoformat(), or datetime.time.fromisoformat() need to be run to successfully transform the value to a datetime, date, or time. I will need to add some regex comparisons to determine which method the value should be run through to be successfully transformed.

@BuzzCutNorman BuzzCutNorman marked this pull request as ready for review November 27, 2023 22:50
@BuzzCutNorman BuzzCutNorman requested review from a team and kgpayne as code owners November 27, 2023 22:50
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b4a7b3e) 87.48% compared to head (fbeff42) 87.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2070      +/-   ##
==========================================
+ Coverage   87.48%   87.50%   +0.01%     
==========================================
  Files          58       58              
  Lines        4875     4882       +7     
  Branches      993      996       +3     
==========================================
+ Hits         4265     4272       +7     
  Misses        429      429              
  Partials      181      181              

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

@edgarrmondragon edgarrmondragon changed the title refactor(targets): Parse dates with backports.datetime_fromisoformat refactor(targets): Parse dates with datetime.fromisoformat/backports.datetime_fromisoformat Nov 27, 2023
Copy link
Collaborator

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

Thanks a bunch @BuzzCutNorman!

@edgarrmondragon edgarrmondragon added this pull request to the merge queue Nov 27, 2023
Merged via the queue into meltano:main with commit ad702b7 Nov 27, 2023
30 checks passed
@BuzzCutNorman BuzzCutNorman deleted the 2046-refactor-parse-dates-with-backports-datetime-fromisoformat branch November 28, 2023 15:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: Parse dates with a more performant library than python-dateutil
2 participants