8000 custom behaviour for timedelta subclasses by samuelcolvin · Pull Request #881 · pydantic/pydantic-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

custom behaviour for timedelta subclasses #881

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 15, 2023
Merged

custom behaviour for timedelta subclasses #881

merged 3 commits into from
Aug 15, 2023

Conversation

samuelcolvin
Copy link
Member
@samuelcolvin samuelcolvin commented Aug 14, 2023 •
edited
Loading

Change Summary

Support pandas Timedelta which don't supply support the Python timedelta C-API.

Related issue number

Fix pydantic/pydantic#7095

Checklist

  • Unit tests for the changes exist
  • Documentation reflects the changes where applicable
  • Pydantic tests pass with this pydantic-core (except for expected changes)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @davidhewitt

@samuelcolvin samuelcolvin marked this pull request as ready for review August 14, 2023 14:35
@samuelcolvin
Copy link
Member Author

please review.

@codecov
Copy link
codecov bot commented Aug 14, 2023 •
edited
Loading

Codecov Report

Merging #881 (68aa3bb) into main (7d67b91) will decrease coverage by 0.02%.
The diff coverage is 98.71%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #881      +/-   ##
==========================================
- Coverage   93.85%   93.84%   -0.02%     
==========================================
  Files         104      104              
  Lines       15387    15391       +4     
  Branches       25       25              
==========================================
+ Hits        14441    14443       +2     
- Misses        940      942       +2     
  Partials        6        6
Files Changed Coverage Δ
src/input/mod.rs 100.00% <ø> (ø)
src/input/datetime.rs 98.67% <96.66%> (-0.21%) ⬇️
src/input/input_python.rs 98.04% <100.00%> (ø)
src/serializers/config.rs 93.68% <100.00%> (+0.20%) ⬆️
src/serializers/infer.rs 95.69% <100.00%> (+0.03%) ⬆️
src/serializers/type_serializers/timedelta.rs 98.03% <100.00%> (+0.16%) ⬆️
src/validators/timedelta.rs 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

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

@codspeed-hq
Copy link
codspeed-hq bot commented Aug 14, 2023 •
edited
Loading

CodSpeed Performance Report

Merging #881 will degrade performances by 24.44%

Comparing timedelta-subclass (68aa3bb) with main (7d67b91)

Summary

❌ 2 regressions
✅ 136 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main timedelta-subclass Change
❌ test_complete_core_lax 1.3 ms 1.5 ms -15.93%
❌ test_decimal_from_string_core 53.4 µs 70.6 µs -24.44%

davidhewitt
davidhewitt approved these changes Aug 15, 2023
View reviewed changes
Copy link
Contributor
@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Overall LGTM, except you'll need to use an older pandas version to make 3.7 happy.

A somewhat related nit is that in idiomatic Rust as_x methods are usually zero cost casts and to_x would be the more correct term for many of the functions implemented here. I might push something to change that separately sometime.

@davidhewitt davidhewitt merged commit 5c98b05 into main Aug 15, 2023
@davidhewitt davidhewitt deleted the timedelta-subclass branch August 15, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt approved these changes

Assignees

@davidhewitt davidhewitt

Labels
ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect JSON output for a timedelta formed from subtracting two pd.Timestamps
2 participants
@samuelcolvin @davidhewitt
0