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
Add option to create directories #155
Add option to create directories #155
Conversation
src/platformdirs/api.py
Outdated
|
||
def _ensure_path_exists(self, path: str) -> None: | ||
if self.ensure_exists and not os.path.exists(path): | ||
Path(path).mkdir(parents=True) |
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.
can't we just pass exists_ok to mkdir and avoid the if?
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.
Good point, I forgot about exists_ok
. But just to make sure I understand, I'll still need to gate this with if self.ensure_exists:
.
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.
yeah, just the os.path.exists is not neeeded
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.
Sounds good. What do you think of the name of the method being _ensure_path_exists()
? I thought that having something more "conditional" would be more clear, but didn't come up with a good alternative.
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'd be ok to drop it and just inline it where needed 🤷
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.
Yeah, I had thought about that, but then one spot I forgot a conditional so I decided having a simple function call was safer.
I've updated it to use exists_ok
and renamed the function based on my own concerns.
Please let me know if there is anything else you'd like to see.
Optionally create directories upon use.
d644f84
to
4663bf8
Compare
@gaborbernat or @smsearcy |
Optionally create directories upon use, based on the
ensure_exists
argument.Fixes #120
While documenting the
ensure_exists
argument on the different functions, I noticed a few spots where theroaming
link was broken, so I fixed those. I can back that out if you'd prefer.