I have looked through the history of shutil.rmtree up until the original contribution by Guido. A few notes:
- ignore_errors argument seems entirely unnecessary as in os.walk it is successfully replaced by onerror. However, it feels like they kept it for backwards compatibility because it was available from the very first draft of shutil.rmtree
- shutil.rmtree is still worked on, as in the example with dir_fd (2 mo ago) by Serhiy Storchaka. It is also pretty darn complex and for a good reason
So I once again argue that if we include it in pathlib, we should wrap shutil. Question to @barneygale and all contributors into pathlib: why doesnāt pathlib use shutil at all? It seems so strange that there must be a good reason.
Either way, Iām gonna write up a pull request for addition of rmtree right now and we can continue from there.
+1. If/when we add an AbstractPath
class, we might want to add a generic implementation using AbstractPath.walk()
etc. But for now we can call straight through to shutil
.
ignore_errors argument seems entirely unnecessary as in os.walk it is successfully replaced by onerror. However, it feels like they kept it for backwards compatibility because it was available from the very first draft of shutil.rmtree
I suppose ignore_errors=True
is easier to write than onerror=lambda a, b, c: pass
?
If onerror is provided, it must be a callable that accepts three parameters: function, path, and excinfo.
This is different from the on_error function for your proposed walk()
method (which only accepts a single parameter) - any way we can reconcile the difference in pathlib?
ignore_errors=True is easier BUT in os.walk we do on_error=None.
Please, take a look at how I handled it in the pull request. I really like this implementation but as always, Iām open to change it.
Docs and tests are only a draft. I really want to add 2 more tests but I havenāt had time to figure out how to test partial removal just yet.
As for your question about onerror signature, we could wrap the userās function to convert args as follows:
def _convert_err(func):
@functools.wraps(func)
def wrapper(exc_type, exc, tb):
return func(exc)
return wrapper
or just go with my current approach and ignore the inconsistency. Though I really donāt want to ignore it.
I messed up the format:
def _convert_err(func):
@functools.wraps(func)
def wrapper(func, path, exc_info):
return func(exc_info[1])
return wrapper
But it feels like theyāre doing it like so for a good reason. It makes for a more precise error handling in more complex situations (because rmtree is fundamentally more complex than, say, walk)