Skip to content

feat(pyarrow) FromPyArrow on Vec<T>: allow any iterable for input#10155

Open
Tpt wants to merge 2 commits into
apache:mainfrom
Tpt:tpt/from-py-arrow-vec
Open

feat(pyarrow) FromPyArrow on Vec<T>: allow any iterable for input#10155
Tpt wants to merge 2 commits into
apache:mainfrom
Tpt:tpt/from-py-arrow-vec

Conversation

@Tpt

@Tpt Tpt commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

This is already the behavior of FromPyObject implementation on Vec<T: FromPyObject>

This is already the behavior of `FromPyObject` implementation on `Vec<T: FromPyObject>`
@Tpt Tpt force-pushed the tpt/from-py-arrow-vec branch from e02a7ae to 66cff0b Compare June 17, 2026 19:42

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @Tpt

Makes sense to me -- I had one question

Also, is there any way to add a test showing this API in action so we can avoid regressions in the future

Comment thread arrow-pyarrow/src/lib.rs
fn from_pyarrow_bound(value: &Bound<PyAny>) -> PyResult<Self> {
let list = value.cast::<PyList>()?;
list.iter().map(|x| T::from_pyarrow_bound(&x)).collect()
let mut v = Vec::with_capacity(value.len().unwrap_or(0));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not keep the collect syntax?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a micro-opt to pre-allocate the Vec at the expected size. .collect() won't have a visibility on it because of PyAny::try_iter. Glad to revert if you prefer

@Tpt Tpt force-pushed the tpt/from-py-arrow-vec branch from 77a5996 to a212868 Compare June 18, 2026 20:06
@Tpt

Tpt commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Thank you!

Also, is there any way to add a test showing this API in action so we can avoid regressions in the future

Done: a212868

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