pitrou commented on code in PR #45549:
URL: https://github.com/apache/arrow/pull/45549#discussion_r1973292656


##########
python/pyarrow/_parquet.pyx:
##########
@@ -739,7 +749,13 @@ cdef class SortingColumn:
 cdef class RowGroupMetaData(_Weakrefable):
     """Metadata for a single row group."""
 
-    def __cinit__(self, FileMetaData parent, int index):
+    def __init__(self):
+        raise TypeError(f"Can't instantiate internal class 
{self.__class__.__name__}")
+
+    def __cinit__(self):
+        pass
+
+    def init(self, FileMetaData parent, int index):

Review Comment:
   Should we make this a `cdef`? It should probably not be exposed in Python.



##########
python/pyarrow/tests/parquet/test_metadata.py:
##########
@@ -67,8 +67,8 @@ def test_parquet_metadata_api():
     assert meta.num_rows == len(df)
     assert meta.num_columns == ncols + 1  # +1 for index
     assert meta.num_row_groups == 1
-    assert meta.format_version == '2.6'
-    assert 'parquet-cpp' in meta.created_by
+    assert meta.format_version == "2.6"

Review Comment:
   Ouch, it seems there are a lot of spurious code formatting changes in this 
file (did you run `black` or a similar utility perhaps?). Can you undo them so 
that they don't pollute the git history?



##########
python/pyarrow/_parquet.pyx:
##########
@@ -52,6 +52,9 @@ _MAX_ROW_GROUP_SIZE = 64*1024*1024
 cdef class Statistics(_Weakrefable):
     """Statistics for a single column in a single row group."""
 
+    def __init__(self):
+        raise TypeError(f"Can't instantiate internal class 
{self.__class__.__name__}")

Review Comment:
   Those classes are not really internal, though, it's just that they shouldn't 
be instantiated directly. Can we change the error message to something else? 
For example:
   ```suggestion
           raise TypeError(f"Do not call {self.__class__.__name__}'s 
constructor directly")
   ```
   



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