Awesome06 commented on issue #55418: URL: https://github.com/apache/arrow/issues/55418#issuecomment-6733177437
I checked the history and implementation of `RecordBatch.from_arrays` to see why `arrays` was typed to `list` in Cython: There does not appear to be an architectural or safety reason for it—it looks to be a historical artifact from 2017. A few details: 1. **Under the hood:** `RecordBatch.from_arrays` only checks `len(arrays)` and `arrays[0]` before passing it to `_sanitize_arrays(arrays, ...)`. `_sanitize_arrays` produces an internal Python list (`converted_arrays`), which is then used to populate the C++ `vector[shared_ptr[CArray]]`. The input sequence is never passed directly to C++. 2. **Parity with `Table`:** `Table.from_arrays` has always kept `arrays` untyped and accepts tuples without issue. 3. **Original intent:** When `pa.record_batch()` was added in ARROW-6263, it was explicitly written as `if isinstance(data, (list, tuple)): return RecordBatch.from_arrays(data, ...)`. Tuples were clearly intended to be supported, but were blocked by the legacy Cython typing. Given that, dropping the `list` restriction on `RecordBatch.from_arrays` (`def from_arrays(arrays, ...)`) should be completely safe and aligns it with `Table.from_arrays` and the typing stubs in #55264. <details> <summary>Commit history details</summary> * **Oct 2016 (`a9747ceac`, ARROW-312):** Both `RecordBatch.from_arrays` and `Table.from_arrays` were originally untyped (`def from_arrays(names, arrays)`). * **Apr 2017 (`7d433dc27`, ARROW-483 / PR #588):** `RecordBatch.from_arrays` had Cython types added (`def from_arrays(list arrays, list names, dict metadata=None)`), while `Table.from_arrays` kept `arrays` untyped. * **Jul 2018 (`ec9b72a23`, ARROW-1715):** `list names` was relaxed to `names` in `RecordBatch.from_arrays` to support schemas during unpickling, leaving `list arrays`. * **Dec 2018 (`9da458437`, ARROW-2624):** `dict metadata` was relaxed to `metadata` to allow arbitrary mappings, leaving `list arrays`. * **Aug 2019 (`05bc63c3b`, ARROW-6263):** `pa.record_batch` and `pa.table` factory functions were added with `isinstance(data, (list, tuple))`. `pa.table` worked because `Table.from_arrays` was untyped, while `pa.record_batch` failed on tuples due to the remaining Cython check. </details> -- 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]
