[GitHub] [arrow] pitrou commented on a change in pull request #10157: ARROW-9299: [C++][Python] Expose ORC metadata

[GitBox <gi...@apache.org>]

pitrou commented on a change in pull request #10157:
URL: https://github.com/apache/arrow/pull/10157#discussion_r629332737



##########
File path: python/pyarrow/includes/common.pxd
##########
@@ -34,6 +34,7 @@ cimport cpython
 
 cdef extern from * namespace "std" nogil:
     cdef shared_ptr[T] static_pointer_cast[T, U](shared_ptr[U])
+    cdef shared_ptr[T] const_pointer_cast[T, U](shared_ptr[U])

Review comment:
       This doesn't seem used?

##########
File path: cpp/src/arrow/adapters/orc/adapter.cc
##########
@@ -229,6 +229,17 @@ class ORCFileReader::Impl {
     return GetArrowSchema(type, out);
   }
 
+  Result<std::shared_ptr<KeyValueMetadata>> ReadMetadata() {
+    std::list<std::string> keys = reader_->getMetadataKeys();

Review comment:
       I meant the list of keys :-)

##########
File path: python/pyarrow/_orc.pyx
##########
@@ -86,14 +107,14 @@ cdef class ORCReader(_Weakrefable):
             int64_t stripe
             std_vector[int] indices
 
-        stripe = n
+        stripe=n

Review comment:
       Why are you making all these changes?

##########
File path: python/pyarrow/orc.py
##########
@@ -26,11 +26,13 @@
 
 
 def _is_map(typ):
-    return (types.is_list(typ) and
-            types.is_struct(typ.value_type) and
-            typ.value_type.num_fields == 2 and
-            typ.value_type[0].name == 'key' and
-            typ.value_type[1].name == 'value')
+    return (
+        types.is_list(typ) and
+        types.is_struct(typ.value_type) and
+        typ.value_type.num_fields == 2 and
+        typ.value_type[0].name == "key" and
+        typ.value_type[1].name == "value"
+    )

Review comment:
       Hmm... can you avoid making unrelated style changes? If your editor is configured to auto-reformat Python code, you may want to disable that.

##########
File path: python/pyarrow/_orc.pyx
##########
@@ -22,16 +22,19 @@
 from cython.operator cimport dereference as deref
 from libcpp.vector cimport vector as std_vector
 from libcpp.utility cimport move
+from libcpp.memory cimport const_pointer_cast

Review comment:
       This doesn't seem used?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org