Skip to content

PyEllipsis and PyNotImplemented new get_bound api#3797

Merged
davidhewitt merged 4 commits intoPyO3:mainfrom
snuderl:PyEllipsis-and-NotImplemented-get-bound-api
Feb 5, 2024
Merged

PyEllipsis and PyNotImplemented new get_bound api#3797
davidhewitt merged 4 commits intoPyO3:mainfrom
snuderl:PyEllipsis-and-NotImplemented-get-bound-api

Conversation

@snuderl
Copy link
Contributor

@snuderl snuderl commented Feb 4, 2024

Parts of #3684

This adds get_bound method to PyEllipsis and PyNotImplemented

@snuderl snuderl force-pushed the PyEllipsis-and-NotImplemented-get-bound-api branch from 0774e25 to 8388b14 Compare February 4, 2024 19:09
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 4, 2024

CodSpeed Performance Report

Merging #3797 will degrade performances by 10.75%

Comparing snuderl:PyEllipsis-and-NotImplemented-get-bound-api (f1384f3) with main (5dbb51b)

Summary

⚡ 3 improvements
❌ 1 regressions
✅ 75 untouched benchmarks

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

Benchmarks breakdown

Benchmark main snuderl:PyEllipsis-and-NotImplemented-get-bound-api Change
mapping_from_dict 327.8 ns 272.2 ns +20.41%
sequence_from_list 355.6 ns 300 ns +18.52%
not_a_list_via_downcast 242.8 ns 215 ns +12.92%
f64_from_pyobject 461.1 ns 516.7 ns -10.75%

@davidhewitt davidhewitt added the CI-skip-changelog Skip checking changelog entry label Feb 4, 2024
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.

Thanks, overall looks good! Just a couple of tiny adjustments that I'd like to see...

(I also notice now that I missed we didn't do this for the PyNone change earlier, it might be nice to go back and do that either as part of this PR or a separate follow-up.)

@davidhewitt davidhewitt marked this pull request as ready for review February 4, 2024 21:06
@snuderl
Copy link
Contributor Author

snuderl commented Feb 5, 2024

Thanks, overall looks good! Just a couple of tiny adjustments that I'd like to see...

(I also notice now that I missed we didn't do this for the PyNone change earlier, it might be nice to go back and do that either as part of this PR or a separate follow-up.)

Done

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.

Perfect, thanks!

deprecated(
since = "0.21.0",
note = "`PyNone::get` will be replaced by `PyBool::get_bound` in a future PyO3 version"
note = "`PyNone::get` will be replaced by `PyNone::get_bound` in a future PyO3 version"
Copy link
Member

Choose a reason for hiding this comment

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

Ah good catch! 👍

@davidhewitt davidhewitt added this pull request to the merge queue Feb 5, 2024
Merged via the queue into PyO3:main with commit c9c6f92 Feb 5, 2024
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.

2 participants