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

Further relax extract_sequence for the upcoming release 0.18 #2632

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions newsfragments/2632.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
`impl FromPyObject for Vec<T>` 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.
3 changes: 1 addition & 2 deletions pytests/tests/test_sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
38 changes: 17 additions & 21 deletions src/types/sequence.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -284,9 +283,6 @@ where
T: FromPyObject<'a>,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
if let Ok(true) = obj.is_instance_of::<PyString>() {
return Err(PyTypeError::new_err("Can't extract `str` to `Vec`"));
}
extract_sequence(obj)
}

Expand All @@ -299,18 +295,9 @@ fn extract_sequence<'s, T>(obj: &'s PyAny) -> PyResult<Vec<T>>
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 {
<PySequence as PyTryFrom>::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::<T>()?);
}
Ok(v)
Expand Down Expand Up @@ -414,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::<Vec<&str>>(py).is_err());
assert!(ob.extract::<Vec<String>>(py).is_err());
assert!(ob.extract::<Vec<char>>(py).is_err());
assert_eq!(
ob.extract::<Vec<&str>>(py).unwrap(),
["L", "o", "n", "d", "o", "n", " ", "C", "a", "l", "l", "i", "n", "g"]
);
assert_eq!(
ob.extract::<Vec<String>>(py).unwrap(),
["L", "o", "n", "d", "o", "n", " ", "C", "a", "l", "l", "i", "n", "g"]
);
assert_eq!(
ob.extract::<Vec<char>>(py).unwrap(),
['L', 'o', 'n', 'd', 'o', 'n', ' ', 'C', 'a', 'l', 'l', 'i', 'n', 'g']
);
});
}

Expand Down