-
Notifications
You must be signed in to change notification settings - Fork 374
Port util_path from ubelt #228
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
Conversation
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.
So far so good! Really nice set of functionality here. Just a few relatively small changes and quesitons, mostly around docstrings. Great work!
boltons/pathutils.py
Outdated
| >>> augpath('foo.tar.gz', ext='.zip', multidot=False) | ||
| foo.tar.zip | ||
| >>> augpath('foo.tar.gz', suffix='_new', multidot=True) | ||
| foo_new.tar.gz |
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.
Ah, thanks for this example. I was wondering what multidot meant practically, and I see it now.
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.
Do you have any ideas for a better name than multidot? I was thinking dots as a shorthand, but I'm not sure if it becomes unclear as to what its doing.
Or perhaps ndots, where you specify the maximum number of dots allowed in an extension?
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.
hmm, been thinking about this one. I can't remember whether I knew the number of dots back when I was splitting extensions last. I could see ndots or maxdots though.
If it stays a bool flag, then the only names that pop into my head are greedy and eager. Multidot isn't that bad though.
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.
Thanks for giving this some thought. I guess it make sense to leave it as a boolean with the current name, given that nothing comes to mind easily.
The main issue I have with the name is that it is 8 characters plus the 5 characters for =True. I would prefer to have something slightly more compact because I often find that setting this flag myself going over the 80 character line limit when I use this function, but that is a first world problem.
I think the other part that bugs me about this is that in general it's not possible to know whether text after the first dot is meant to be a file extension or is actually part of the file name. Sure there are heuristics (e.g. look for .tar.gz), but these break down easily (e.g. .json.custom.backup). I also agree that I typically don't know the exact number of dots that are possible I'm expecting when processing a filepath, but I typically will know if dots are allowed in the filename or not, so it might be better to opt for the simpler boolean API.
|
Hey @Erotemic just saw that the last commit was " |
|
This is pretty much ready to go. The "wip" is because I'm often lazy with my commit messages ( |
|
Excellent, merged! I'll try to do one last release in 2019, but otherwise this may be boltons==20.0.0! :) |
In #220 you mentioned
compressuser, which I've renamed toshrinkuser(when I looked up "opposite of expand" in google, shrink was the first answer). I also selected several other related functions from ubelt.util_path and I omitted ones that had less usage.The 4 new functions introduced with brief justifications for their inclusion are:
shrinkuser- the inverse ofos.path.expanduser. Sometimes you want to transform a path to be relative to the user directory on any machine. Its also a more concise representation.expandpath- os.path hasexpanduserandexpandvars, but should a user really need to call two functions to expand strings that might start with~or$HOME? There should be a single function for tat.userhome- The directory of the user directory. I'm not sure how useful this is by itself, but it is used to implementshrinkuserand it seems like a reasonable stdliby like thing.augpath- Usingjoin(spit(p)[0], splitext(basename(p))[0] + '.txt')is really ugly, whereasaugpath(p, ext='.txt')is an extremely concise way to specific modify parts of a filepath. This is a common task that should have a builtin function. I do think there is room for discussion on the name of themultidotkeyword argument. I'm open to renaming that. I use this function often and every time I needed to setmutltidot=Trueit felt cumbersome. Maybe something likedots=Trueto indicate the extension may contain more than one dot?Let me know what you think of the functionality and then I'll add CHANGELOG / other bookkeeping as necessary.