danepitkin commented on code in PR #431:
URL: https://github.com/apache/arrow-nanoarrow/pull/431#discussion_r1581480452


##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -967,10 +989,50 @@ cdef class CSchemaBuilder:
         if self._ptr.release == NULL:
             ArrowSchemaInit(self._ptr)
 
+    @staticmethod
+    def copy_existing(CSchema existing_schema):
+        return CSchemaBuilder(existing_schema.__deepcopy__())

Review Comment:
   nit: I'd prefer a shorter name. WDYT? 
   ```suggestion
       def copy(CSchema schema):
           return CSchemaBuilder(schema.__deepcopy__())
   ```



##########
python/src/nanoarrow/iterator.py:
##########
@@ -160,6 +164,17 @@ def get_iterator(cls, obj, schema=None):
                 yield from iterator._iter1(0, array.length)
 
     def _iter1(self, offset, length):
+        # Check for and extension type first since this isn't reflected by

Review Comment:
   ```suggestion
           # Check for an extension type first since this isn't reflected by
   ```



##########
python/src/nanoarrow/schema.py:
##########
@@ -108,10 +115,73 @@ def create(obj):
         return TimeUnit(obj)
 
 
+class ExtensionAccessor:
+    """Accessor for extension type parameters"""
+
+    def __init__(self, schema) -> None:
+        self._schema = schema
+
+    @property
+    def name(self) -> str:
+        """Extension name for this extension type"""
+        return self._schema._c_schema_view.extension_name
+
+    @property
+    def metadata(self) -> Union[bytes, None]:
+        """Extension metadata for this extension type if present"""
+        extension_metadata = self._schema._c_schema_view.extension_metadata
+        return extension_metadata if extension_metadata else None
+
+    @property
+    def storage(self):
+        """Storage type for this extension type"""
+        metadata = dict(self._schema.metadata.items())
+        del metadata[b"ARROW:extension:name"]
+        if b"ARROW:extension:metadata" in metadata:
+            del metadata[b"ARROW:extension:metadata"]

Review Comment:
   why do we delete the objects here?



##########
python/src/nanoarrow/schema.py:
##########
@@ -108,10 +115,73 @@ def create(obj):
         return TimeUnit(obj)
 
 
+class ExtensionAccessor:
+    """Accessor for extension type parameters"""
+
+    def __init__(self, schema) -> None:
+        self._schema = schema
+
+    @property
+    def name(self) -> str:
+        """Extension name for this extension type"""
+        return self._schema._c_schema_view.extension_name
+
+    @property
+    def metadata(self) -> Union[bytes, None]:
+        """Extension metadata for this extension type if present"""
+        extension_metadata = self._schema._c_schema_view.extension_metadata
+        return extension_metadata if extension_metadata else None
+
+    @property
+    def storage(self):
+        """Storage type for this extension type"""
+        metadata = dict(self._schema.metadata.items())
+        del metadata[b"ARROW:extension:name"]
+        if b"ARROW:extension:metadata" in metadata:
+            del metadata[b"ARROW:extension:metadata"]
+
+        return Schema(self._schema, metadata=metadata)
+
+
 class Schema:
-    """The Schema is nanoarrow's high-level data type representation whose 
scope maps to
-    that of the ArrowSchema in the Arrow C Data interface. See :func:`schema` 
for class
-    details.
+    """Create a nanoarrow Schema
+
+    The Schema is nanoarrow's high-level data type representation, encompasing

Review Comment:
   ```suggestion
       The Schema is nanoarrow's high-level data type representation, 
encompassing
   ```



##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -967,10 +989,50 @@ cdef class CSchemaBuilder:
         if self._ptr.release == NULL:
             ArrowSchemaInit(self._ptr)
 
+    @staticmethod
+    def copy_existing(CSchema existing_schema):
+        return CSchemaBuilder(existing_schema.__deepcopy__())
+
     @staticmethod
     def allocate():
         return CSchemaBuilder(CSchema.allocate())
 
+    def clear_metadata(self):

Review Comment:
   Optional: What if we made this more object oriented? e.g. 
`CSchemaBuilder.clear_metadata()` vs `CSchemaBuilder.metadata.clear()`. Same 
with `append_metadata`. Does it make sense to implement this way? If not (or 
its a lot of work), ignore this comment.



-- 
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]

Reply via email to