Walk iterators (#134)

* iterator walking

* docstring

* refinement

* simplify walk_depth

* tweak

* scrutinizer complaint

* tidy

* tweak

* simplification of walk

* tidy

* more efficient depth walking, refinements

* simplification

* depth fix

* simplify

* optimize copy and test

* optimized copying

* added getfile to multi and mount

* changelog

* indent

.travis.yml

@@ -9,7 +9,6 @@ python:
 
 os:
   - linux
-  - osx
 
 before_install:
   - pip install setuptools pip -U

CHANGELOG.md

@@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - fs.getfile function
 
+### Changed
+
+- Modified walk to use iterators internally (for more efficient walking)
+- Modified fs.copy to use getfile
+
 ## [2.0.17] - 2017-11-20
 
 ### Fixed

fs/compress.py

@@ -102,7 +102,6 @@ def write_zip(src_fs,
                     _zip.write(sys_path, zip_name)
 
 
-
 def write_tar(src_fs,
               file,
               compression=None,

fs/copy.py

@@ -103,10 +103,39 @@ def copy_file(src_fs, src_path, dst_fs, dst_path):
             else:
                 # Standard copy
                 with src_fs.lock(), dst_fs.lock():
-                    with src_fs.openbin(src_path) as read_file:
-                        # There may be an optimized copy available on
-                        # dst_fs
-                        dst_fs.setbinfile(dst_path, read_file)
+                    if dst_fs.hassyspath(dst_path):
+                        with dst_fs.openbin(dst_path, 'w') as write_file:
+                            src_fs.getfile(src_path, write_file)
+                    else:
+                        with src_fs.openbin(src_path) as read_file:
+                            dst_fs.setbinfile(dst_path, read_file)
+
+
+def copy_file_internal(src_fs, src_path, dst_fs, dst_path):
+    """Low level copy, that doesn't call manage_fs or lock.
+
+    If the destination exists, and is a file, it will be first truncated.
+
+    This method exists to optimize copying in loops. In general you
+    should prefer `copy_file`.
+
+    Arguments:
+        src_fs (FS or str): Source filesystem.
+        src_path (str): Path to a file on the source filesystem.
+        dst_fs (FS or str): Destination filesystem.
+        dst_path (str): Path to a file on the destination filesystem.
+
+    """
+    if src_fs is dst_fs:
+        # Same filesystem, so we can do a potentially optimized
+        # copy
+        src_fs.copy(src_path, dst_path, overwrite=True)
+    elif dst_fs.hassyspath(dst_path):
+        with dst_fs.openbin(dst_path, 'w') as write_file:
+            src_fs.getfile(src_path, write_file)
+    else:
+        with src_fs.openbin(src_path) as read_file:
+            dst_fs.setbinfile(dst_path, read_file)
 
 
 def copy_file_if_newer(src_fs, src_path, dst_fs, dst_path):
@@ -143,11 +172,8 @@ def copy_file_if_newer(src_fs, src_path, dst_fs, dst_path):
                 with src_fs.lock(), dst_fs.lock():
                     if _source_is_newer(src_fs, src_path,
                                         dst_fs, dst_path):
-                        with src_fs.openbin(src_path) as read_file:
-                            # There may be an optimized copy available
-                            # on dst_fs
-                            dst_fs.setbinfile(dst_path, read_file)
-                            return True
+                        copy_file_internal(src_fs, src_path, dst_fs, dst_path)
+                        return True
                     else:
                         return False
 
@@ -209,7 +235,7 @@ def copy_dir(src_fs, src_path, dst_fs, dst_path,
                     for info in files:
                         src_path = info.make_path(dir_path)
                         dst_path = info.make_path(copy_path)
-                        copy_file(
+                        copy_file_internal(
                             src_fs,
                             src_path,
                             dst_fs,
@@ -276,5 +302,5 @@ def copy_dir_if_newer(src_fs, src_path, dst_fs, dst_path,
                             src_modified > dst_state[dir_path].modified
                         )
                         if do_copy:
-                            copy_file(src_fs, dir_path, dst_fs, copy_path)
+                            copy_file_internal(src_fs, dir_path, dst_fs, copy_path)
                             on_copy(src_fs, dir_path, dst_fs, copy_path)

fs/mirror.py

@@ -20,7 +20,7 @@
 from __future__ import unicode_literals
 
 
-from .copy import copy_file
+from .copy import copy_file_internal
 from .errors import ResourceNotFound
 from .walk import Walker
 from .opener import manage_fs
@@ -92,7 +92,7 @@ def _mirror(src_fs, dst_fs, walker=None, copy_if_newer=True):
                     # Compare file info
                     if copy_if_newer and not _compare(_file, dst_file):
                         continue
-            copy_file(src_fs, _path, dst_fs, _path)
+            copy_file_internal(src_fs, _path, dst_fs, _path)
 
         # Make directories
         for _dir in dirs:

fs/mountfs.py

@@ -159,6 +159,10 @@ def getbytes(self, path):
         fs, _path = self._delegate(path)
         return fs.getbytes(_path)
 
+    def getfile(self, path, file, chunk_size=None, **options):
+        fs, _path = self._delegate(path)
+        return fs.getfile(_path, file, chunk_size=chunk_size, **options)
+
     def gettext(self, path, encoding=None, errors=None, newline=None):
         self.check()
         fs, _path = self._delegate(path)

fs/multifs.py

@@ -259,6 +259,10 @@ def getbytes(self, path):
             raise errors.ResourceNotFound(path)
         return fs.getbytes(path)
 
+    def getfile(self, path, file, chunk_size=None, **options):
+        fs = self._delegate_required(path)
+        return fs.getfile(path, file, chunk_size=chunk_size, **options)
+
     def gettext(self, path, encoding=None, errors=None, newline=''):
         self.check()
         fs = self._delegate_required(path)
@@ -291,13 +295,13 @@ def geturl(self, path, purpose='download'):
 
     def hassyspath(self, path):
         self.check()
-        fs = self._delegate_required(path)
-        return fs.hassyspath(path)
+        fs = self._delegate(path)
+        return fs is not None and fs.hassyspath(path)
 
     def hasurl(self, path, purpose='download'):
         self.check()
-        fs = self._delegate_required(path)
-        return fs.hasurl(path, purpose=purpose)
+        fs = self._delegate(path)
+        return fs is not None and fs.hasurl(path, purpose=purpose)
 
     def isdir(self, path):
         self.check()

fs/test.py

@@ -401,6 +401,8 @@ def test_getsyspath(self):
             self.assertFalse(self.fs.hassyspath('foo'))
         else:
             self.assertTrue(self.fs.hassyspath('foo'))
+        # Should not throw an error
+        self.fs.hassyspath('a/b/c/foo/bar')
 
     def test_geturl(self):
         self.fs.create('foo')
@@ -410,6 +412,8 @@ def test_geturl(self):
             self.assertFalse(self.fs.hasurl('foo'))
         else:
             self.assertTrue(self.fs.hasurl('foo'))
+        # Should not throw an error
+        self.fs.hasurl('a/b/c/foo/bar')
 
     def test_geturl_purpose(self):
         """Check an unknown purpose raises a NoURL error.
@@ -1075,8 +1079,14 @@ def test_removedir(self):
 
     def test_removetree(self):
         self.fs.makedirs('foo/bar/baz')
+        self.fs.makedirs('foo/egg')
+        self.fs.makedirs('foo/a/b/c/d/e')
         self.fs.create('foo/egg.txt')
         self.fs.create('foo/bar/egg.bin')
+        self.fs.create('foo/bar/baz/egg.txt')
+        self.fs.create('foo/a/b/c/1.txt')
+        self.fs.create('foo/a/b/c/2.txt')
+        self.fs.create('foo/a/b/c/3.txt')
 
         self.assert_exists('foo/egg.txt')
         self.assert_exists('foo/bar/egg.bin')