-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support moving across filesystems in pathlib.Path, as shutil.move() does #73991
Comments
Trying to use Pathlib and Path.replace on Windows if drive are different leads to an issue:
This is a known situation of os.rename, and workaround I found is to use shutil or to copy/delete manually in two steps (e.g. http://stackoverflow.com/questions/21116510/python-oserror-winerror-17-the-system-cannot-move-the-file-to-a-different-d) When using Pathlib, it's not that easy to workaround using shutil (even if thanks to Brett Cannon now shutil accepts Path in Py3.6, not everybody has Py3.6). At least this should be documented with a recommendation for that situation. I love Pathlib and it's too bad my code becomes complicated when it was so simple :( |
Just to confirm, I was able to workaround it with Py3.6: # client_generated_path.replace(destination_folder)
shutil.move(client_generated_path, destination_folder) It is reasonable to think about adding a similar feature to pathlib if it doesn't support it and just special-case the drive-to-drive scenario for Windows |
Moving a file across volumes isn't atomic. Getting an exception in this case can be useful. There could be a "strict" keyword-only parameter that defaults to False. If it's true, then replace() won't try to move the file. |
I agree this needs to be different from replace(), due to not being atomic. That makes it an enhancement, so I'm removing 3.5. I'm +1 on Path supporting something like shutil.move(). |
I also support the idea of getting something like shutil.move() into pathlib. |
I should also mention that rename() (https://docs.python.org/3/library/pathlib.html#pathlib.Path.rename) and replace() (https://docs.python.org/3/library/pathlib.html#pathlib.Path.replace) already do exist, so it might be best to add a keyword-only flag to one of those for this use-case. |
I'm also +1 on adding def move(src, dst, copy_function=copy2):
return pathlib.Path(src).move(dst, copy_function=copy2) I'm interested in this approach because I'm hoping to add a To achieve this we'd need to make pathlib accept We'd also need to move other shutil functions that On |
Sounds reasonable to me, though would we want to include |
I think @barneygale 's plan is sound. |
I agree with @zooba, exposing the |
I agree about def copy(self, dst, *, follow_symlinks=True, copy_mode=True, copy_stat=False):
...
def move(self, dst, *, atomic=False, **kwargs):
if atomic:
return os.rename(self, dst)
self.copy(dst, **kwargs)
self.unlink() |
I don't like the For example, on Windows we explicitly disallow If we were to define semantics for We should make these functions an improvement in semantics, not just discoverability, and ideally more appropriate for whatever platform they're being used on than the POSIX semantics inherent to posixmodule. So I think the only option we want on Similarly, I think we should simplify Footnotes
|
My example was a bit rubbish - here's a corrected version: def copy(self, dst, *, follow_symlinks=True, copy_mode=True, copy_stat=True):
...
def move(self, dst, *, atomic=False, **kwargs):
try:
return os.rename(self, dst)
except OSError:
if atomic:
raise # Move can't be achieved atomically
self.copy(dst, **kwargs)
self.unlink() So I think I agree with you on removing copy_mode and copy_stat. |
I suppose one could argue that folks should just use |
Yeah, I'll argue that one ;) We'll have to explain that |
Should we deprecate |
Good point - perhaps not! |
Rename `pathlib.Path.rmtree()` to `delete()`, and add support for deleting non-directories. This simplifies the interface for users, and nicely complements the upcoming `move()` and `copy()` methods (which will also accept any type of file.)
Rename `pathlib.Path.copy()` to `_copy_file()` (i.e. make it private.) Rename `pathlib.Path.copytree()` to `copy()`, and add support for copying non-directories. This simplifies the interface for users, and nicely complements the upcoming `move()` and `delete()` methods (which will also accept any type of file.)
Rename `pathlib.Path.rmtree()` to `delete()`, and add support for deleting non-directories. This simplifies the interface for users, and nicely complements the upcoming `move()` and `copy()` methods (which will also accept any type of file.)
Rename `pathlib.Path.copy()` to `_copy_file()` (i.e. make it private.) Rename `pathlib.Path.copytree()` to `copy()`, and add support for copying non-directories. This simplifies the interface for users, and nicely complements the upcoming `move()` and `delete()` methods (which will also accept any type of file.) Co-authored-by: Adam Turner <[email protected]>
Remove the *ignore_errors* and *on_error* arguments from `Path.delete()`. This functionality was carried over from `shutil`, but its design needs to be re-considered in its new context. For example, we may wish to support a *missing_ok* argument (like `Path.unlink()`), or automatically `chmod()` and retry operations when we hit a permission error (like `tempfile.TemporaryDirectory`), or retry operations with a backoff (like `test.support.os_helper.rmtree()`), or utilise exception groups, etc. It's best to leave our options open for now.
…n#122368) Rename `pathlib.Path.rmtree()` to `delete()`, and add support for deleting non-directories. This simplifies the interface for users, and nicely complements the upcoming `move()` and `copy()` methods (which will also accept any type of file.)
…n#122369) Rename `pathlib.Path.copy()` to `_copy_file()` (i.e. make it private.) Rename `pathlib.Path.copytree()` to `copy()`, and add support for copying non-directories. This simplifies the interface for users, and nicely complements the upcoming `move()` and `delete()` methods (which will also accept any type of file.) Co-authored-by: Adam Turner <[email protected]>
Add a `Path.move()` method that moves a file or directory tree, and returns a new `Path` instance pointing to the target. This method is similar to `shutil.move()`, except that it doesn't accept a *copy_function* argument, and it doesn't check whether the destination is an existing directory.
These two methods accept an *existing* directory path, onto which we join the source path's base name to form the final target path. A possible alternative implementation is to check for directories in `copy()` and `move()` and adjust the target path, which is done in several `shutil` functions. This behaviour is helpful in a shell context, but less so in a stored program that explicitly specifies destinations. For example, a user that calls `Path('foo.py').copy('bar.py')` might not imagine that `bar.py/foo.py` would be created, but under the alternative implementation this will happen if `bar.py` is an existing directory.
Per feedback from Paul Moore on pythonGH-123158, it's better to defer making `Path.delete()` public than ship it with under-designed error handling capabilities. We leave a remnant `_delete()` method, which is used by `move()`. Any functionality not needed by `move()` is deleted.
These two methods accept an *existing* directory path, onto which we join the source path's base name to form the final target path. A possible alternative implementation is to check for directories in `copy()` and `move()` and adjust the target path, which is done in several `shutil` functions. This behaviour is helpful in a shell context, but less so in a stored program that explicitly specifies destinations. For example, a user that calls `Path('foo.py').copy('bar.py')` might not imagine that `bar.py/foo.py` would be created, but under the alternative implementation this will happen if `bar.py` is an existing directory.
Remove *ignore* and *on_error* arguments from `pathlib.Path.copy[_into]()`, because these arguments are under-designed. Specifically: - *ignore* is appropriated from `shutil.copytree()`, but it's not clear how it should apply when the user copies a non-directory. We've changed the callback signature from the `shutil` version, but I'm not confident the new signature is as good as it can be. - *on_error* is a generalisation of `shutil.copytree()`'s error handling, which is to accumulate exceptions and raise a single `shutil.Error` at the end. It's not obvious which solution is better. Additionally, this arguments may be challenging to implement in future user subclasses of `PathBase`, which might utilise a native recursive copying method.
Per feedback from Paul Moore on GH-123158, it's better to defer making `Path.delete()` public than ship it with under-designed error handling capabilities. We leave a remnant `_delete()` method, which is used by `move()`. Any functionality not needed by `move()` is deleted.
…23337) Remove *ignore* and *on_error* arguments from `pathlib.Path.copy[_into]()`, because these arguments are under-designed. Specifically: - *ignore* is appropriated from `shutil.copytree()`, but it's not clear how it should apply when the user copies a non-directory. We've changed the callback signature from the `shutil` version, but I'm not confident the new signature is as good as it can be. - *on_error* is a generalisation of `shutil.copytree()`'s error handling, which is to accumulate exceptions and raise a single `shutil.Error` at the end. It's not obvious which solution is better. Additionally, this arguments may be challenging to implement in future user subclasses of `PathBase`, which might utilise a native recursive copying method.
It's done! Thank you everyone who helped formulate and review this feature. We've added new The new methods expect exact targets (or target directories for the The Feedback most welcome! We have ~9 months until 3.14 beta 1 where the interface is more locked down, so there's still plenty of time make changes. I'll close this issue, but I can re-open it (or log new issues) if problems are identified. Thanks again, all. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
pathlib.Path.copy()
#119058pathlib.Path.rmtree()
#119060os.copy()
and friends #119079shutil._rmtree_[un]safe()
. #120517pathlib.Path.copy()
#120519DummyPath.unlink()
andrmdir()
#120715pathlib.Path.copytree()
#120718pathlib.Path.copy()
#120806pathlib.Path.copytree()
#121438pathlib.Path.move()
#122073pathlib.Path.rmtree()
intodelete()
#122368pathlib.Path.copytree()
intocopy()
#122369pathlib.Path.copy()
#122924pathlib.Path.delete()
arguments #123158pathlib.Path.copy_into()
andmove_into()
#123314pathlib.Path.delete()
private. #123315pathlib.Path.copy()
andcopy_into()
arguments #123337The text was updated successfully, but these errors were encountered: