Skip to content

Use UserWarning from pyo3 #1559

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
merged 2 commits into from
Dec 3, 2024
Merged

Use UserWarning from pyo3 #1559

merged 2 commits into from
Dec 3, 2024

Conversation

changhc
Copy link
Contributor

@changhc changhc commented Nov 29, 2024

Change Summary

Following David's comment: #1551 (review)

Replacing the explicitly imported python UserWarning with PyUserWarning from pyo3. This might slightly improve performance in this case, depending on how pyo3 actually raises warnings. In the worst case it should be the same as the current implementation, so there's no harm doing this.

Related issue number

None.

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: @sydney-runkle

Copy link

codspeed-hq bot commented Nov 29, 2024

CodSpeed Performance Report

Merging #1559 will not alter performance

Comparing changhc:user-warning (5630fa4) with main (ab0e4bd)

Summary

✅ 155 untouched benchmarks

Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.63%. Comparing base (ab503cb) to head (5630fa4).
Report is 249 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1559      +/-   ##
==========================================
- Coverage   90.21%   89.63%   -0.59%     
==========================================
  Files         106      112       +6     
  Lines       16339    17889    +1550     
  Branches       36       40       +4     
==========================================
+ Hits        14740    16034    +1294     
- Misses       1592     1835     +243     
- Partials        7       20      +13     
Files with missing lines Coverage Δ
src/serializers/extra.rs 96.56% <100.00%> (-2.97%) ⬇️

... and 57 files 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 ab0e4bd...5630fa4. Read the comment docs.

@changhc changhc marked this pull request as ready for review December 3, 2024 08:34
@changhc
Copy link
Contributor Author

changhc commented Dec 3, 2024

please review

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.

Thanks for following up 👍

@davidhewitt davidhewitt merged commit be87e05 into pydantic:main Dec 3, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants