alamb commented on code in PR #11161: URL: https://github.com/apache/arrow-rs/pull/11161#discussion_r4190371876
########## arrow-array/src/array_reader.rs: ########## @@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Traits for reading a stream of [`ArrayRef`] that share a single [`FieldRef`]. + +use arrow_schema::{ArrowError, FieldRef}; + +use crate::array::ArrayRef; + +/// Trait for types that can read `ArrayRef`'s. +/// +/// This is the array-level counterpart to [`RecordBatchReader`], for streams whose +/// elements are not record batches. +/// +/// To create from an iterator, see [ArrayIterator]. +/// +/// [`RecordBatchReader`]: crate::RecordBatchReader +pub trait ArrayReader: Iterator<Item = Result<ArrayRef, ArrowError>> { + /// Returns the field of this `ArrayReader`. + /// + /// Implementation of this trait should guarantee that all `ArrayRef`'s returned by this + /// reader should have the same field as returned from this method. Review Comment: The double "should" could be reduced: ```suggestion /// reader have the same field as returned from this method. ``` ########## arrow-array/src/array_reader.rs: ########## @@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Traits for reading a stream of [`ArrayRef`] that share a single [`FieldRef`]. + +use arrow_schema::{ArrowError, FieldRef}; + +use crate::array::ArrayRef; + +/// Trait for types that can read `ArrayRef`'s. +/// +/// This is the array-level counterpart to [`RecordBatchReader`], for streams whose +/// elements are not record batches. +/// +/// To create from an iterator, see [ArrayIterator]. +/// +/// [`RecordBatchReader`]: crate::RecordBatchReader +pub trait ArrayReader: Iterator<Item = Result<ArrayRef, ArrowError>> { Review Comment: name is unused 👍 https://docs.rs/arrow/latest/arrow/index.html?search=ArrayReader ########## arrow-array/src/array_reader.rs: ########## @@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Traits for reading a stream of [`ArrayRef`] that share a single [`FieldRef`]. + +use arrow_schema::{ArrowError, FieldRef}; + +use crate::array::ArrayRef; + +/// Trait for types that can read `ArrayRef`'s. +/// +/// This is the array-level counterpart to [`RecordBatchReader`], for streams whose +/// elements are not record batches. +/// +/// To create from an iterator, see [ArrayIterator]. +/// +/// [`RecordBatchReader`]: crate::RecordBatchReader +pub trait ArrayReader: Iterator<Item = Result<ArrayRef, ArrowError>> { + /// Returns the field of this `ArrayReader`. + /// + /// Implementation of this trait should guarantee that all `ArrayRef`'s returned by this + /// reader should have the same field as returned from this method. + fn field(&self) -> &FieldRef; +} + +impl<R: ArrayReader + ?Sized> ArrayReader for Box<R> { + fn field(&self) -> &FieldRef { + self.as_ref().field() + } +} + +/// Generic implementation of [ArrayReader] that wraps an iterator. +/// +/// # Example +/// +/// ``` +/// # use std::sync::Arc; +/// # use arrow_array::{ArrayIterator, ArrayReader, ArrayRef, Int32Array}; +/// # use arrow_schema::{DataType, Field}; +/// # +/// let a: ArrayRef = Arc::new(Int32Array::from(vec![1, 2])); +/// let b: ArrayRef = Arc::new(Int32Array::from(vec![3, 4])); +/// +/// let field = Arc::new(Field::new("values", DataType::Int32, false)); +/// let mut reader = ArrayIterator::new(vec![a.clone(), b.clone()].into_iter().map(Ok), field.clone()); +/// +/// assert_eq!(reader.field(), &field); +/// assert_eq!(&reader.next().unwrap().unwrap(), &a); +/// # assert_eq!(&reader.next().unwrap().unwrap(), &b); +/// # assert!(reader.next().is_none()); +/// ``` +pub struct ArrayIterator<I> Review Comment: similarly, available: https://docs.rs/arrow/latest/arrow/index.html?search=ArrayIterator ########## arrow-array/src/ffi_stream.rs: ########## @@ -21,14 +21,15 @@ //! One interface maps C ABI to native Rust types, i.e. convert c-pointers, c_char, to native rust. //! This is handled by [FFI_ArrowArrayStream]. //! -//! The second interface is used to import `FFI_ArrowArrayStream` as Rust implementation `RecordBatch` reader. -//! This is handled by `ArrowArrayStreamReader`. +//! The second interface is used to import `FFI_ArrowArrayStream` as a Rust reader. A stream +//! whose top-level type is `Struct` can be read as `RecordBatch` by `RecordBatchStreamReader`; +//! a stream of any type can be read as `ArrayRef` by `ArrayStreamReader`. Review Comment: can we please make `ArrayStreamReader` a doc reference? [`ArrayStreamReader`] ? ########## arrow-array/src/ffi_stream.rs: ########## @@ -404,11 +407,15 @@ fn get_error_code(err: &ArrowError) -> i32 { /// Its main responsibility is to expose `RecordBatchReader` functionality /// that requires [FFI_ArrowArrayStream]. #[derive(Debug)] -pub struct ArrowArrayStreamReader { +pub struct RecordBatchStreamReader { Review Comment: 👍 ########## arrow-array/src/ffi_stream.rs: ########## @@ -404,11 +407,15 @@ fn get_error_code(err: &ArrowError) -> i32 { /// Its main responsibility is to expose `RecordBatchReader` functionality /// that requires [FFI_ArrowArrayStream]. #[derive(Debug)] -pub struct ArrowArrayStreamReader { +pub struct RecordBatchStreamReader { stream: FFI_ArrowArrayStream, schema: SchemaRef, } +/// Renamed to [`RecordBatchStreamReader`], to distinguish it from [`ArrayStreamReader`]. +#[deprecated(since = "60.1.0", note = "Use `RecordBatchStreamReader` instead")] Review Comment: nice ########## arrow-array/src/array_reader.rs: ########## @@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Traits for reading a stream of [`ArrayRef`] that share a single [`FieldRef`]. + +use arrow_schema::{ArrowError, FieldRef}; + +use crate::array::ArrayRef; + +/// Trait for types that can read `ArrayRef`'s. +/// +/// This is the array-level counterpart to [`RecordBatchReader`], for streams whose +/// elements are not record batches. +/// +/// To create from an iterator, see [ArrayIterator]. +/// +/// [`RecordBatchReader`]: crate::RecordBatchReader +pub trait ArrayReader: Iterator<Item = Result<ArrayRef, ArrowError>> { + /// Returns the field of this `ArrayReader`. + /// + /// Implementation of this trait should guarantee that all `ArrayRef`'s returned by this + /// reader should have the same field as returned from this method. + fn field(&self) -> &FieldRef; +} + +impl<R: ArrayReader + ?Sized> ArrayReader for Box<R> { + fn field(&self) -> &FieldRef { + self.as_ref().field() + } +} + +/// Generic implementation of [ArrayReader] that wraps an iterator. +/// +/// # Example +/// +/// ``` +/// # use std::sync::Arc; +/// # use arrow_array::{ArrayIterator, ArrayReader, ArrayRef, Int32Array}; +/// # use arrow_schema::{DataType, Field}; +/// # +/// let a: ArrayRef = Arc::new(Int32Array::from(vec![1, 2])); +/// let b: ArrayRef = Arc::new(Int32Array::from(vec![3, 4])); +/// +/// let field = Arc::new(Field::new("values", DataType::Int32, false)); +/// let mut reader = ArrayIterator::new(vec![a.clone(), b.clone()].into_iter().map(Ok), field.clone()); Review Comment: Possibly easier to understand: ```suggestion /// let iter = vec![a.clone(), b.clone()].into_iter().map(Ok); /// let mut reader = ArrayIterator::new(iter, field.clone()); ``` ########## arrow-array/src/array_reader.rs: ########## @@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Traits for reading a stream of [`ArrayRef`] that share a single [`FieldRef`]. + +use arrow_schema::{ArrowError, FieldRef}; + +use crate::array::ArrayRef; + +/// Trait for types that can read `ArrayRef`'s. +/// +/// This is the array-level counterpart to [`RecordBatchReader`], for streams whose +/// elements are not record batches. +/// +/// To create from an iterator, see [ArrayIterator]. +/// +/// [`RecordBatchReader`]: crate::RecordBatchReader +pub trait ArrayReader: Iterator<Item = Result<ArrayRef, ArrowError>> { + /// Returns the field of this `ArrayReader`. + /// + /// Implementation of this trait should guarantee that all `ArrayRef`'s returned by this + /// reader should have the same field as returned from this method. + fn field(&self) -> &FieldRef; +} + +impl<R: ArrayReader + ?Sized> ArrayReader for Box<R> { + fn field(&self) -> &FieldRef { + self.as_ref().field() + } +} + +/// Generic implementation of [ArrayReader] that wraps an iterator. +/// +/// # Example +/// +/// ``` +/// # use std::sync::Arc; +/// # use arrow_array::{ArrayIterator, ArrayReader, ArrayRef, Int32Array}; +/// # use arrow_schema::{DataType, Field}; +/// # +/// let a: ArrayRef = Arc::new(Int32Array::from(vec![1, 2])); +/// let b: ArrayRef = Arc::new(Int32Array::from(vec![3, 4])); +/// +/// let field = Arc::new(Field::new("values", DataType::Int32, false)); +/// let mut reader = ArrayIterator::new(vec![a.clone(), b.clone()].into_iter().map(Ok), field.clone()); +/// +/// assert_eq!(reader.field(), &field); +/// assert_eq!(&reader.next().unwrap().unwrap(), &a); +/// # assert_eq!(&reader.next().unwrap().unwrap(), &b); Review Comment: why hide these? ########## arrow-array/src/ffi_stream.rs: ########## Review Comment: What do you think about cross referencing `ArrayReader` here? like "See [`ArrayReader`] for reading single arays" or something And we could add similar "see also" comment to `ArrayReader` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
