paleolimbot commented on code in PR #464:
URL: https://github.com/apache/arrow-nanoarrow/pull/464#discussion_r1603610335
##########
python/src/nanoarrow/visitor.py:
##########
@@ -124,27 +242,26 @@ def finish(self) -> Any:
return None
-class ListBuilder(ArrayStreamVisitor):
- def __init__(self, schema, *, iterator_cls=PyIterator, array_view=None):
+class SingleColumnBuilder(ArrayStreamVisitor):
+ def __init__(self, schema, handle_nulls=None, *, array_view=None):
super().__init__(schema, array_view=array_view)
-
- # Ensure that self._iterator._array_view is self._array_view
- self._iterator = iterator_cls(schema, array_view=self._array_view)
+ cls, kwargs = _resolve_column_builder_cls(
+ self._schema, handle_nulls=handle_nulls
+ )
+ self._visitor = cls(schema, **kwargs, array_view=self._array_view)
def begin(self, total_elements: Union[int, None] = None):
- self._lst = []
+ self._visitor.begin(total_elements)
- def visit_chunk_view(self, array_view: CArrayView):
- # The constructor here ensured that self._iterator._array_view
- # is populated when self._set_array() is called.
- self._lst.extend(self._iterator)
+ def visit_chunk_view(self, array_view: CArrayView) -> None:
+ self._visitor.visit_chunk_view(array_view)
- def finish(self) -> List:
- return self._lst
+ def finish(self) -> Any:
+ return self._visitor.finish()
class ColumnsBuilder(ArrayStreamVisitor):
Review Comment:
It's been through a few iterations of naming 🙂 . Maybe `sequence` is better
than `column`? The gist of it is "something that either numpy or Pandas can
understand" and/or "we dealt with all the hard parts of a non-contiguous or
non-zero offset array so you didn't have to". I could stick with `Visitor`
instead of `Builder`, too (but these are internal for now so at least that part
can get renamed).
--
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]
