Skip to content

Use the new bound API instead of .as_ref(py)#3853

Merged
davidhewitt merged 8 commits intoPyO3:mainfrom
LilyFirefly:replace-as-ref-with-bind
Feb 18, 2024
Merged

Use the new bound API instead of .as_ref(py)#3853
davidhewitt merged 8 commits intoPyO3:mainfrom
LilyFirefly:replace-as-ref-with-bind

Conversation

@LilyFirefly
Copy link
Contributor

I was looking into the step of marking Py::as_ref as deprecated and while there's still some blockers, I was able to update to use the new api in a bunch of places.

@davidhewitt davidhewitt added the CI-skip-changelog Skip checking changelog entry label Feb 17, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 17, 2024

CodSpeed Performance Report

Merging #3853 will degrade performances by 16.65%

Comparing LilyFoote:replace-as-ref-with-bind (1a98cbd) with main (c33d330)

🎉 Hooray! codspeed-rust just leveled up to 2.3.3!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

⚡ 2 improvements
❌ 2 regressions
✅ 75 untouched benchmarks

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

Benchmarks breakdown

Benchmark main LilyFoote:replace-as-ref-with-bind Change
mapping_from_dict 355.6 ns 300 ns +18.52%
extract_float_downcast_success 425.6 ns 508.9 ns -16.38%
extract_float_extract_success 417.2 ns 500.6 ns -16.65%
sequence_from_list 355.6 ns 300 ns +18.52%

`to_str` is not available before Python 3.10 on the limited api.
Copy link
Member

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

Looks good to me! I marked a few spots, where I think we can relax lifetime constraints.

Copy link
Member

@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.

Nice to clean these up, thanks! Just a few small thoughts... 👍

Copy link
Member

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

Nice tidy up, LGTM 🚀

Copy link
Member

@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.

Great, thanks! 🚀

@davidhewitt davidhewitt added this pull request to the merge queue Feb 18, 2024
Merged via the queue into PyO3:main with commit 0dd568d Feb 18, 2024
@LilyFirefly LilyFirefly deleted the replace-as-ref-with-bind branch February 18, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-skip-changelog Skip checking changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants