jorisvandenbossche commented on code in PR #445:
URL: https://github.com/apache/arrow-nanoarrow/pull/445#discussion_r1584732649
##########
python/src/nanoarrow/schema.py:
##########
@@ -957,14 +988,113 @@ def struct(fields, nullable=True) -> Schema:
>>> import nanoarrow as na
>>> na.struct([na.int32()])
Schema(STRUCT, fields=[Schema(INT32)])
- >>> na.struct([("col1", na.int32())])
- Schema(STRUCT, fields=[Schema(INT32, name='col1')])
>>> na.struct({"col1": na.int32()})
Schema(STRUCT, fields=[Schema(INT32, name='col1')])
"""
return Schema(Type.STRUCT, fields=fields, nullable=nullable)
+def list_of(value_type, nullable=True) -> Schema:
Review Comment:
Is there reason you went with the `_of` pattern for those types to avoid a
clash with the built-in `list` for this one?
The alternative that pyarrow uses is just for `list` append an underscore to
the function name. Also not really nice, but somewhat standard practice (I have
to say the `na.list_of(na.int64())` actually reads fine once seeing it in code,
but for looking for the types in the namespace I find that it looks a bit
weird, and is different in how other Arrow implementations call this)
##########
python/src/nanoarrow/schema.py:
##########
@@ -343,6 +347,34 @@ def scale(self) -> int:
return self._c_schema_view.decimal_scale
+ @property
+ def index_type(self):
Review Comment:
Maybe use "dictionary" in the name? A generic `index_type` could be a bit
confusing.
(although for other type-specific attributes we also don't include the type
in the name, like precision and scale etc, so this is maybe consistent ..)
##########
python/src/nanoarrow/schema.py:
##########
@@ -343,6 +347,34 @@ def scale(self) -> int:
return self._c_schema_view.decimal_scale
+ @property
+ def index_type(self):
Review Comment:
Can you also add some docstrings to the new properties?
##########
python/src/nanoarrow/schema.py:
##########
@@ -945,9 +977,8 @@ def struct(fields, nullable=True) -> Schema:
----------
fields :
* A dictionary whose keys are field names and values are schema-like
objects
- * An iterable whose items are a schema like object or a two-tuple of
the
- field name and a schema-like object. If a field name is not specified
- from the tuple, the field name is inherited from the schema-like
object.
+ * An iterable whose items are a schema like objects where the field
name is
+ inherited from the schema-like object.
Review Comment:
That ability of passing tuples is removed? (don't directly see that in the
diff)
##########
python/src/nanoarrow/schema.py:
##########
@@ -945,9 +977,8 @@ def struct(fields, nullable=True) -> Schema:
----------
fields :
* A dictionary whose keys are field names and values are schema-like
objects
- * An iterable whose items are a schema like object or a two-tuple of
the
- field name and a schema-like object. If a field name is not specified
- from the tuple, the field name is inherited from the schema-like
object.
+ * An iterable whose items are a schema like objects where the field
name is
+ inherited from the schema-like object.
Review Comment:
(Ah, i see now, it's below in the diff)
--
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]
