accept any iterator for PySet::new and PyFrozenSet::new
#2795
Conversation
src/types/frozenset.rs
Outdated
| pub fn new<'p, T: ToPyObject>(py: Python<'p>, elements: &[T]) -> PyResult<&'p PyFrozenSet> { | ||
| let list = elements.to_object(py); | ||
| unsafe { py.from_owned_ptr_or_err(ffi::PyFrozenSet_New(list.as_ptr())) } | ||
| pub fn new<'p, T: ToPyObject>( |
There was a problem hiding this comment.
Just for the record, this is breaking since &[T] is IntoIterator<Item = &T>.
There was a problem hiding this comment.
Hmm. I think in which case let's use &T here for now too. We can revisit that as a different breaking change in future.
There was a problem hiding this comment.
If we're going to be breaking code anyway, what about taking IntoPy instead? i.e. inserting pyclasses into the set by value?
There was a problem hiding this comment.
Yes, I think I'd like to do this (and I've thought similar in the past for PyList::new). I will hold off from doing that here, as this is again non-breaking after I've corrected to use Item = &T for now.
I'd be keen to revisit this along with #2316 and also a design about how we maybe also handle fallible into-py conversions.
|
bors r=birkenfeld |
|
Build succeeded: |
Closes #2789
Looks like it's about 20% faster in the case I benchmarked to use
PySet_Adddirectly without the intermediate list.