Skip to content
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

Add TryExtend trait #4664

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

bschoenmaeckers
Copy link
Contributor

This PR adds a TryExtend trait that is similar to the std::iter::Extend trait except it is failable. Currently implemented for PyList & PyDict, including some optimised specializations on nightly.

@@ -0,0 +1,23 @@
//! A trait for extending a collection with elements from an iterator, returning an error if the operation fails.
Copy link
Contributor Author

@bschoenmaeckers bschoenmaeckers Oct 28, 2024

Choose a reason for hiding this comment

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

I could not find a obvious file to add this trait to so I created a new one. Suggestions are welcome.

@bschoenmaeckers bschoenmaeckers force-pushed the try-extend branch 4 times, most recently from 1a4ce61 to 3cd380f Compare October 28, 2024 17:29
@mejrs
Copy link
Member

mejrs commented Oct 28, 2024

I'm unsure whether I'm in favor of this, but I don't want specialization. We used to rely on it but removed it a couple of years ago. The problem with specialization (even the min version) is that it has soundness issues when it comes to lifetimes and it also has no realistic path towards stabilization.

Also, you can add a compatibility shim for PyList_Extend in the compat module.

@bschoenmaeckers
Copy link
Contributor Author

Alternatively I could add 2 methods to PyList & PyDict; extend_from_pyobject and extend_from_iter. This does not require specialization and is optimized for both cases.

Copy link

codspeed-hq bot commented Oct 29, 2024

CodSpeed Performance Report

Merging #4664 will improve performances by 16.34%

Comparing bschoenmaeckers:try-extend (06d8289) with main (2d3bdc0)

Summary

⚡ 5 improvements
✅ 78 untouched benchmarks

Benchmarks breakdown

Benchmark main bschoenmaeckers:try-extend Change
test_empty_class_init_py 15.3 µs 13.8 µs +10.85%
test_args_kwargs_py 21.1 µs 18.5 µs +14.27%
test_args_kwargs_rs 15.8 µs 13.6 µs +16.34%
test_simple_kwargs_py 22.3 µs 19.9 µs +12.46%
test_simple_kwargs_rs 21.6 µs 18.7 µs +15.28%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants