Repository navigation
Special case for "" & "." in posixpath.abspath() #117639
Description
Activity
How often does real code call
abspathon the current working directory? I'm somewhat sceptical that making the code more complicated here is worth it for such an edge caseReacted by Barney Gale, Kerim Kabirov and Erlend E. AaslandThis speeds up
posixpath.relpath()with 1 argument at no cost.This speeds up
posixpath.relpath()with 1 argument at no cost.I see. That would be useful information to edit into the issue description.
- changed the title
[-]Special case for current dir in `posixpath.abspath()`[/-][+]Special case for "" & "." in `posixpath.abspath()`[/+]on Apr 9, 2024 This speeds up
posixpath.relpath()with 1 argument at no cost.How much is that speedup? According to your PR there is no speedup for the general call with one argument.
I'll do a benchmark soon. The pull request currently doesn't compare the performance of
posixpath.relpath(), but it will be at most 13% faster.This special case makes
posixpath.relpath()with a single argument 4-6% faster.This special case makes
posixpath.relpath()with a single argument 4-6% faster.Hmm. In my opinion, this particular optimisation is not worth it for a speedup of that magnitude. The refactor makes the code of this function look quite odd (the kind of thing that somebody keen on "refactoring" or "simplifying" the code of
os.pathmight propose rewriting in a PR), and it's quite unobvious to a reader of the code that it's been deliberately written that way in order to speeduprelpath().Reacted by Erlend E. AaslandOK, I was planning for a larger speedup anyway, that would apply to all relative paths: #117587.
Feature or enhancement
Proposal:
We could add a special case for the current directory and return it without normalising. This speeds up
posixpath.relpath()with one argument.def abspath(path): """Return an absolute path.""" path = os.fspath(path) if isinstance(path, bytes): - if not path.startswith(b'/'): - path = join(os.getcwdb(), path) + sep = b'/' + curdir = b'.' + getcwd = os.getcwdb else: - if not path.startswith('/'): - path = join(os.getcwd(), path) + sep = '/' + curdir = '.' + getcwd = os.getcwd + if not path.startswith(sep): + if not path or path == curdir: + return getcwd() + path = join(getcwd(), path) return normpath(path)Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
os.path#117610Linked PRs
posixpath.abspath()#117640