Skip to content

add bound constructors for PySet and PyFrozenSet #3743

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
Jan 27, 2024

Conversation

davidhewitt
Copy link
Member

Implements new_bound for PySet and PyFrozenSet, adding the deprecations to new and updating relevant internal code.

@davidhewitt davidhewitt added CI-build-full CI-skip-changelog Skip checking changelog entry and removed CI-build-full labels Jan 17, 2024
Copy link

codspeed-hq bot commented Jan 17, 2024

CodSpeed Performance Report

Merging #3743 will degrade performances by 10.44%

Comparing davidhewitt:set-bound-constructors (4e24e68) with main (43504cd)

Summary

⚡ 3 improvements
❌ 1 regressions
✅ 74 untouched benchmarks

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

Benchmarks breakdown

Benchmark main davidhewitt:set-bound-constructors Change
extract_int_downcast_fail 238.3 ns 266.1 ns -10.44%
iter_set 51.9 ms 40.7 ms +27.41%
not_a_list_via_downcast 153.9 ns 126.1 ns +22.03%
list_via_downcast 153.9 ns 126.1 ns +22.03%

@davidhewitt davidhewitt force-pushed the set-bound-constructors branch from e8d40d6 to 06c9543 Compare January 17, 2024 09:45
@davidhewitt
Copy link
Member Author

Unless anyone feels a strong desire to review this one, I might merge this and also #3755 in the near future, as both are mechanical changes just following the patterns we've already established on how we want these new_bound APIs to be like for now.

Copy link
Member

@adamreichold adamreichold left a comment

Choose a reason for hiding this comment

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

The diff is a bit messy, but the result looks good to me.

@davidhewitt
Copy link
Member Author

Thanks! Yes, I think quite a few of these PRs adding / deprecating constructors are touching quite a few bits of the codebase. It's going to be a relief to tidy these up again in the future.

@davidhewitt davidhewitt added this pull request to the merge queue Jan 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 27, 2024
@davidhewitt davidhewitt added this pull request to the merge queue Jan 27, 2024
Merged via the queue into PyO3:main with commit 0973da2 Jan 27, 2024
@davidhewitt davidhewitt deleted the set-bound-constructors branch January 27, 2024 18:31
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