jorisvandenbossche commented on code in PR #39825:
URL: https://github.com/apache/arrow/pull/39825#discussion_r1470718399


##########
python/pyarrow/fs.py:
##########
@@ -123,15 +123,6 @@ def _ensure_filesystem(
                 return LocalFileSystem(use_mmap=use_mmap)
             return PyFileSystem(FSSpecHandler(filesystem))
 
-    # map old filesystems to new ones
-    import pyarrow.filesystem as legacyfs
-
-    if isinstance(filesystem, legacyfs.LocalFileSystem):
-        return LocalFileSystem(use_mmap=use_mmap)
-    # TODO handle HDFS?
-    if allow_legacy_filesystem and isinstance(filesystem, legacyfs.FileSystem):

Review Comment:
   You can remove the ``allow_legacy_filesystem`` keyword argument to this 
function now



##########
python/pyarrow/parquet/core.py:
##########
@@ -1125,9 +1119,7 @@ def _get_pandas_index_columns(keyvalues):
 
 
 def _is_local_file_system(fs):
-    return isinstance(fs, LocalFileSystem) or isinstance(
-        fs, legacyfs.LocalFileSystem
-    )
+    return isinstance(fs, LocalFileSystem)

Review Comment:
   Given we only use this in one place and it is shorter now, I would just 
inline the `isinstance(fs, LocalFileSystem)`



##########
python/pyarrow/__init__.py:
##########
@@ -252,7 +252,7 @@ def print_entry(label, value):
                          create_memory_map, MockOutputStream,
                          input_stream, output_stream)
 
-from pyarrow._hdfsio import HdfsFile, have_libhdfs
+from pyarrow._hdfsio import have_libhdfs

Review Comment:
   It seems we forgot to explicitly deprecate accessing HdfsFile, but I guess 
everybody doing that would also access the filesystem itself, and should have 
seen the warning.
   
   I suppose `have_libhdfs` still has its use for the non-legacy 
pyarrow.fs.HadoopFileSystem (eg we use it in their tests)



##########
python/pyarrow/_hdfsio.pyx:
##########
@@ -20,16 +20,8 @@
 
 # cython: language_level = 3
 
-import re
-
-from pyarrow.lib cimport check_status, _Weakrefable, NativeFile
-from pyarrow.includes.common cimport *
-from pyarrow.includes.libarrow cimport *
+from pyarrow.lib cimport check_status
 from pyarrow.includes.libarrow_fs cimport *
-from pyarrow.lib import frombytes, tobytes, ArrowIOError
-
-
-_HDFS_PATH_RE = re.compile(r'hdfs://(.*):(\d+)(.*)')
 
 
 def have_libhdfs():

Review Comment:
   If this is the only remaining thing in this file, we can maybe move it 
elsewhere (maybe io.pxi?)



##########
python/pyarrow/tests/parquet/test_dataset.py:
##########
@@ -582,25 +561,41 @@ def _visit_level(base_dir, level, part_keys):
                 str(base_dir),
                 '{}={}'.format(name, value)
             ])
-            fs.mkdir(level_dir)
+            try:
+                fs.create_dir(level_dir)
+            except AttributeError:
+                fs.mkdir(level_dir)

Review Comment:
   Is this `except` case still needed?



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