From 462084243fe84120b1187e0b7cdf79c75c2a8af1 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Tue, 13 Sep 2022 22:05:12 +0200 Subject: [PATCH 1/3] Further relax extract_sequence as neither PyObject::len nor PyObject::iter actually need the sequence protocol. --- src/types/sequence.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 9fa4010c732..d7878b8a209 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -299,18 +299,9 @@ fn extract_sequence<'s, T>(obj: &'s PyAny) -> PyResult> where T: FromPyObject<'s>, { - // Types that pass `PySequence_Check` usually implement enough of the sequence protocol - // to support this function and if not, we will only fail extraction safely. - let seq = unsafe { - if ffi::PySequence_Check(obj.as_ptr()) != 0 { - ::try_from_unchecked(obj) - } else { - return Err(PyDowncastError::new(obj, "Sequence").into()); - } - }; - - let mut v = Vec::with_capacity(seq.len().unwrap_or(0)); - for item in seq.iter()? { + let iter = obj.iter()?; + let mut v = Vec::with_capacity(obj.len().unwrap_or(0)); + for item in iter { v.push(item?.extract::()?); } Ok(v) From 18232ba0212e099301b12083aff15c6dbcc9654d Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 21 Sep 2022 10:10:39 +0200 Subject: [PATCH 2/3] Revert #2500, i.e. impl FromPyObject for Vec will accept str. --- pytests/tests/test_sequence.py | 3 +-- src/types/sequence.rs | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/pytests/tests/test_sequence.py b/pytests/tests/test_sequence.py index 91aac50ac72..dc44a50defc 100644 --- a/pytests/tests/test_sequence.py +++ b/pytests/tests/test_sequence.py @@ -17,8 +17,7 @@ def test_vec_from_bytes(): def test_vec_from_str(): - with pytest.raises(TypeError): - sequence.vec_to_vec_pystring("123") + assert sequence.vec_to_vec_pystring("123") == ["1", "2", "3"] @pytest.mark.skipif( diff --git a/src/types/sequence.rs b/src/types/sequence.rs index d7878b8a209..a7ed74b246c 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -1,11 +1,10 @@ // Copyright (c) 2017-present PyO3 Project and Contributors use crate::err::{self, PyDowncastError, PyErr, PyResult}; -use crate::exceptions::PyTypeError; use crate::inspect::types::TypeInfo; use crate::internal_tricks::get_ssize_index; use crate::once_cell::GILOnceCell; use crate::type_object::PyTypeInfo; -use crate::types::{PyAny, PyList, PyString, PyTuple, PyType}; +use crate::types::{PyAny, PyList, PyTuple, PyType}; use crate::{ffi, PyNativeType, ToPyObject}; use crate::{AsPyPointer, IntoPy, IntoPyPointer, Py, Python}; use crate::{FromPyObject, PyTryFrom}; @@ -284,9 +283,6 @@ where T: FromPyObject<'a>, { fn extract(obj: &'a PyAny) -> PyResult { - if let Ok(true) = obj.is_instance_of::() { - return Err(PyTypeError::new_err("Can't extract `str` to `Vec`")); - } extract_sequence(obj) } @@ -405,14 +401,23 @@ mod tests { } #[test] - fn test_strings_cannot_be_extracted_to_vec() { + fn test_strings_can_be_extracted_to_vec() { Python::with_gil(|py| { let v = "London Calling"; let ob = v.to_object(py); - assert!(ob.extract::>(py).is_err()); - assert!(ob.extract::>(py).is_err()); - assert!(ob.extract::>(py).is_err()); + assert_eq!( + ob.extract::>(py).unwrap(), + ["L", "o", "n", "d", "o", "n", " ", "C", "a", "l", "l", "i", "n", "g"] + ); + assert_eq!( + ob.extract::>(py).unwrap(), + ["L", "o", "n", "d", "o", "n", " ", "C", "a", "l", "l", "i", "n", "g"] + ); + assert_eq!( + ob.extract::>(py).unwrap(), + ['L', 'o', 'n', 'd', 'o', 'n', ' ', 'C', 'a', 'l', 'l', 'i', 'n', 'g'] + ); }); } From 035af829232f218375a407095e6813f9ced2914a Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 21 Sep 2022 10:11:12 +0200 Subject: [PATCH 3/3] Add changelog entry detailing how much further impl FromPyObject for Vec was relaxed. --- newsfragments/2632.changed.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 newsfragments/2632.changed.md diff --git a/newsfragments/2632.changed.md b/newsfragments/2632.changed.md new file mode 100644 index 00000000000..8a957fa0ba7 --- /dev/null +++ b/newsfragments/2632.changed.md @@ -0,0 +1,11 @@ +`impl FromPyObject for Vec` will accept any Python object that can be turned into an iterator via Python's built-in `iter` function. It will also not reject `str` anymore which can be iterated as a sequence of `str` objects, i.e. [#2500](https://github.com/PyO3/pyo3/pull/2500) was reverted. Please use a type like + +```rust +#[derive(FromPyObject)] +pub enum OneOrMany<'py> { + One(&'py PyString), + Many(Vec<&'py PyString>), +} +``` + +if you would like to work around this edge case for callers of your API.