Home Original page

Move dir optimization by tfeldmann · Pull Request #550 · PyFilesystem/pyfilesystem2

@tfeldmann

Type of changes

  • New feature
  • Tests

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @PyFilesystem/maintainers may be pedantic in the code review.

Description

  • Use shutil.move to move folders if both filesystems have a syspath
  • Check for ResourceNotFound, DirectoryExpected and IllegalDestination in fs.move.move_dir

@coveralls

Coverage Status

Coverage decreased (-0.0003%) to 94.814% when pulling 7c34e41 on tfeldmann:move_dir-optimization into 59f6e4d on PyFilesystem:master.

@tfeldmann

Once again I should rebase this for fewer commits. And it seems to have some problems on windows.

@tfeldmann

@tfeldmann

@tfeldmann

@tfeldmann

This works now, but I'm kind of unhappy with it. For example I'm missing a param overwrite in move.move_dir. Is it ok for you if I add one? It would have to default to true to be backwards compatible.

lurch

with _src_fs.lock(), _dst_fs.lock():
with convert_os_errors("move_dir", dst_path, directory=True):
if _dst_fs.exists(dst_path):
os.rmdir(dst_syspath)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we don't need / want this os.rmdir?
Looking at the non-optimised "standard copy then delete" code, it looks like perhaps if dst_path already exists, and contains existing contents, then the existing contents will be preserved after the move_dir call? 🤷
Certainly something that it's probably worth adding an extra test for 😉