[GitHub] [arrow] pitrou commented on a change in pull request #12231: ARROW-14783: [C++][Python] Fix the BytesIO support issue

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

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



##########
File path: python/pyarrow/orc.py
##########
@@ -22,6 +22,7 @@
 from pyarrow.lib import Table
 import pyarrow._orc as _orc
 from pyarrow.fs import _resolve_filesystem_and_path
+from pyarrow import filesystem as legacyfs

Review comment:
       Why are you importing the old filesystem API here?

##########
File path: cpp/src/arrow/adapters/orc/adapter.cc
##########
@@ -720,11 +720,7 @@ class ArrowOutputStream : public liborc::OutputStream {
     return filename;
   }
 
-  void close() override {
-    if (!output_stream_.closed()) {
-      ORC_THROW_NOT_OK(output_stream_.Close());
-    }
-  }
+  void close() override {}

Review comment:
       There's nothing to do on the ORC side? No resources to release?

##########
File path: python/pyarrow/orc.py
##########
@@ -234,8 +238,11 @@ class ORCWriter:
 """.format(_orc_writer_args_docs)
 
     is_open = False
+    file_handle = None
 
-    def __init__(self, where, *, file_version='0.12',
+    def __init__(self, where, *,
+                 filesystem=None,

Review comment:
       Instead of adding a `filesystem` argument, please add something like a `close_file=False` argument that will select whether to close the given file on `__exit__`.




-- 
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: github-unsubscribe@arrow.apache.org

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