diff --git a/docs/source/copying.rst b/docs/source/copying.rst index e72c628c7..895fcde18 100644 --- a/docs/source/copying.rst +++ b/docs/source/copying.rst @@ -121,9 +121,6 @@ Forward slashes are used for directory separators throughout. .. warning:: - ``recursive=False`` is not correct - (`issue 1232 `_). - ``maxdepth`` is not yet implemented for copying functions (`issue 1231 `_). @@ -175,9 +172,6 @@ Forward slashes are used for directory separators throughout. .. warning:: - ``recursive=False`` is not correct - (`issue 1232 `_). - ``maxdepth`` is not yet implemented for copying functions (`issue 1231 `_). diff --git a/fsspec/implementations/local.py b/fsspec/implementations/local.py index 08d0c5dec..01a50ee42 100644 --- a/fsspec/implementations/local.py +++ b/fsspec/implementations/local.py @@ -281,7 +281,7 @@ def trailing_sep(path): # TODO: if all incoming paths were posix-compliant then separator would # always be a forward slash, simplifying this function. # See https://github.com/fsspec/filesystem_spec/pull/1250 - return path.endswith(os.sep) or os.altsep and path.endswith(os.altsep) + return path.endswith(os.sep) or (os.altsep is not None and path.endswith(os.altsep)) def trailing_sep_maybe_asterisk(path): @@ -294,10 +294,8 @@ def trailing_sep_maybe_asterisk(path): # TODO: if all incoming paths were posix-compliant then separator would # always be a forward slash, simplifying this function. # See https://github.com/fsspec/filesystem_spec/pull/1250 - return ( - path.endswith((os.sep, os.sep + "*")) - or os.altsep - and path.endswith((os.altsep, os.altsep + "*")) + return path.endswith((os.sep, os.sep + "*")) or ( + os.altsep is not None and path.endswith((os.altsep, os.altsep + "*")) ) diff --git a/fsspec/implementations/tests/test_local.py b/fsspec/implementations/tests/test_local.py index 72a2d6eff..c035f40ae 100644 --- a/fsspec/implementations/tests/test_local.py +++ b/fsspec/implementations/tests/test_local.py @@ -939,32 +939,24 @@ def test_cp_get_put_empty_directory(tmpdir, funcname): # cp/get/put without slash, target directory exists assert fs.isdir(target) func(empty, target) - assert fs.find(target, withdirs=True) == [ - make_path_posix(os.path.join(target, "empty")) - ] - - fs.rm(target + "/empty", recursive=True) + assert fs.find(target, withdirs=True) == [] # cp/get/put with slash, target directory exists assert fs.isdir(target) func(empty + "/", target) assert fs.find(target, withdirs=True) == [] - fs.rm(target, recursive=True) + fs.rmdir(target) # cp/get/put without slash, target directory doesn't exist assert not fs.isdir(target) func(empty, target) - assert fs.isdir(target) - assert fs.find(target, withdirs=True) == [] - - fs.rm(target, recursive=True) + assert not fs.isdir(target) # cp/get/put with slash, target directory doesn't exist assert not fs.isdir(target) func(empty + "/", target) - assert fs.isdir(target) - assert fs.find(target, withdirs=True) == [] + assert not fs.isdir(target) def test_cp_two_files(tmpdir): diff --git a/fsspec/implementations/tests/test_memory.py b/fsspec/implementations/tests/test_memory.py index 28f28bf83..7e734da5f 100644 --- a/fsspec/implementations/tests/test_memory.py +++ b/fsspec/implementations/tests/test_memory.py @@ -291,30 +291,24 @@ def test_cp_empty_directory(m): # cp without slash, target directory exists assert m.isdir(target) m.cp(empty, target) - assert m.find(target, withdirs=True) == [target + "/empty"] - - m.rm(target + "/empty", recursive=True) + assert m.find(target, withdirs=True) == [] # cp with slash, target directory exists assert m.isdir(target) m.cp(empty + "/", target) assert m.find(target, withdirs=True) == [] - m.rm(target, recursive=True) + m.rmdir(target) # cp without slash, target directory doesn't exist assert not m.isdir(target) m.cp(empty, target) - assert m.isdir(target) - assert m.find(target, withdirs=True) == [] - - m.rm(target, recursive=True) + assert not m.isdir(target) # cp with slash, target directory doesn't exist assert not m.isdir(target) m.cp(empty + "/", target) - assert m.isdir(target) - assert m.find(target, withdirs=True) == [] + assert not m.isdir(target) def test_cp_two_files(m): diff --git a/fsspec/spec.py b/fsspec/spec.py index 0eddea47f..6add403df 100644 --- a/fsspec/spec.py +++ b/fsspec/spec.py @@ -886,13 +886,21 @@ def get(self, rpath, lpath, recursive=False, callback=_DEFAULT_CALLBACK, **kwarg trailing_sep_maybe_asterisk, ) + rpaths = self.expand_path(rpath, recursive=recursive) + if ( + len(rpaths) == 1 + and not recursive + and (trailing_sep(rpaths[0]) or self.isdir(rpaths[0])) + ): + # Non-recursive copy of directory does nothing. + return + source_is_str = isinstance(rpath, str) + if isinstance(lpath, str): lpath = make_path_posix(lpath) - rpaths = self.expand_path(rpath, recursive=recursive) isdir = isinstance(lpath, str) and ( trailing_sep(lpath) or LocalFileSystem().isdir(lpath) ) - source_is_str = isinstance(rpath, str) lpaths = other_paths( rpaths, lpath, @@ -942,17 +950,25 @@ def put(self, lpath, rpath, recursive=False, callback=_DEFAULT_CALLBACK, **kwarg trailing_sep_maybe_asterisk, ) + if isinstance(lpath, str): + lpath = make_path_posix(lpath) + fs = LocalFileSystem() + lpaths = fs.expand_path(lpath, recursive=recursive) + if ( + len(lpaths) == 1 + and not recursive + and (trailing_sep(lpaths[0]) or self.isdir(lpaths[0])) + ): + # Non-recursive copy of directory does nothing. + return + source_is_str = isinstance(lpath, str) + rpath = ( self._strip_protocol(rpath) if isinstance(rpath, str) else [self._strip_protocol(p) for p in rpath] ) - if isinstance(lpath, str): - lpath = make_path_posix(lpath) - fs = LocalFileSystem() - lpaths = fs.expand_path(lpath, recursive=recursive) isdir = isinstance(rpath, str) and (trailing_sep(rpath) or self.isdir(rpath)) - source_is_str = isinstance(lpath, str) rpaths = other_paths( lpaths, rpath, @@ -996,8 +1012,16 @@ def copy(self, path1, path2, recursive=False, on_error=None, **kwargs): on_error = "raise" paths = self.expand_path(path1, recursive=recursive) - isdir = isinstance(path2, str) and (trailing_sep(path2) or self.isdir(path2)) + if ( + len(paths) == 1 + and not recursive + and (trailing_sep(paths[0]) or self.isdir(paths[0])) + ): + # Non-recursive copy of directory does nothing. + return source_is_str = isinstance(path1, str) + + isdir = isinstance(path2, str) and (trailing_sep(path2) or self.isdir(path2)) path2 = other_paths( paths, path2, @@ -1019,6 +1043,7 @@ def expand_path(self, path, recursive=False, maxdepth=None, **kwargs): kwargs are passed to ``glob`` or ``find``, which may in turn call ``ls`` """ + if maxdepth is not None and maxdepth < 1: raise ValueError("maxdepth must be at least 1") diff --git a/fsspec/tests/abstract/copy.py b/fsspec/tests/abstract/copy.py index 1460e63b4..13f0ccdfc 100644 --- a/fsspec/tests/abstract/copy.py +++ b/fsspec/tests/abstract/copy.py @@ -92,9 +92,8 @@ def test_copy_directory_to_existing_directory( t = target + "/" if target_slash else target # Without recursive does nothing - # ERROR: erroneously creates new directory - # fs.cp(s, t) - # assert fs.ls(target) == [] + fs.cp(s, t) + assert fs.ls(target) == [] # With recursive fs.cp(s, t, recursive=True) @@ -143,9 +142,8 @@ def test_copy_directory_to_new_directory( t += "/" # Without recursive does nothing - # ERROR: erroneously creates new directory - # fs.cp(s, t) - # assert fs.ls(target) == [] + fs.cp(s, t) + assert fs.ls(target) == [] # With recursive fs.cp(s, t, recursive=True)