Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
bpo-33123: pathlib: Add missing_ok parameter to Path.unlink#6191
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
the-knights-who-say-ni commentedMar 22, 2018
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed thePSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please followthe steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
2e2506a to6d6f2e9CompareCLA is signed and submitted. |
Lib/test/test_pathlib.py Outdated
| @@ -1627,6 +1627,11 @@ def test_unlink(self): | |||
| self.assertFileNotFound(p.stat) | |||
| self.assertFileNotFound(p.unlink) | |||
| def test_unlink_missing_ok(self): | |||
| p = self.cls(BASE) / 'fileMissing' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think it's worth changing the name of the file to something sort of explicit such asfileMissing. I'd rather not surprise the reader and use what seems the default in the tests file ie.fileA here too.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Agreed.fileA actually exists, but I changed tofileAAA which seems to be used for a non-existing file in other tests.
Fixes bpo-33123
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A simple enough addition. LGTM.
@rbu: Status check is done, and it's a success ✅ . |
Uh oh!
There was an error while loading.Please reload this page.
Similarly to how several pathlib file creation functions have an "exists_ok" parameter, we should introduce "missing_ok" that makes removal functions not raise an exception when a file or directory is already absent. IMHO, this should cover Path.unlink and Path.rmdir. Note, Path.resolve() has a "strict" parameter since 3.6 that does the same thing. Naming this of this new parameter tries to be consistent with the "exists_ok" parameter as that is more explicit about what it does (as opposed to "strict").
https://bugs.python.org/issue33123