-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
splitext performances improvement #36353
Comments
After more thought, I must admit that the behavior change in splitext, I proposed with patch 536120 is not acceptable. So I would instead propose this one which should only improve performances without modifying behavior. The diff patch also test_posixpath.py to check the pitfall described by Tim comments in patch 536120 page. def splitext(p):
root, ext = '', ''
for c in p:
if c == '/':
root, ext = root + ext + c, ''
elif c == '.':
if ext:
root, ext = root + ext, c
else:
ext = c
elif ext:
ext = ext + c
else:
root = root + c
return root, ext
def splitext2(p):
i = p.rfind('.')
if i<=p.rfind('/'):
return p, ''
else:
return p[:i], p[i:]
l1 = ('t','.t','a.b/','a.b','/a.b','a.b/.c','a.b/c.d')
l2 = (
'usr/tmp.doc/list/home/sebastien/foo/bar/hghgt/yttyutyuyuttyuyut.tyyttyt',
'usr/tmp.doc/list/home/sebastien/foo/bar/hghgt/yttyutyuyuttyuyut.',
'usr/tmp.doc/list/home/sebastien/foo/bar/hghgt/.tyyttyt',
'usr/tmp.doc/list/home/sebastien/foo/bar/hghgt/yttyutyuyuttyuyut',
'reeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeyttyutyuyuttyuyut.tyyttyt',
'/iuouiiuuoiiuiikhjzekezhjzekejkejkzejkhejkhzejzehjkhjezhjkehzkhjezh.tyyttyt'
)
for i in l1+l2:
assert splitext2(i) == splitext(i)
import time
def test(f,args):
t = time.clock()
for p in args:
for i in range(1000):
f(p)
return time.clock() - t
def f(p):pass
a=test(splitext, l1)
b=test(splitext2, l1)
c=test(f,l1)
print a,b,c,(a-c)/(b-c)
a=test(splitext, l2)
b=test(splitext2, l2)
c=test(f,l2)
print a,b,c,(a-c)/(b-c) |
Logged In: YES The patch looks good to me. |
Logged In: YES I like it fine so far as it goes, but I'd like it a lot |
Logged In: YES I have take a look at macpath, dospath and ntpath. I have found quite a lot of code duplication. What would be your opinion, if I tried to do a little refactoring on this? |
1 similar comment
Logged In: YES I have take a look at macpath, dospath and ntpath. I have found quite a lot of code duplication. What would be your opinion, if I tried to do a little refactoring on this? |
Logged In: YES Sharing code is a good thing. However, it would be critical |
Logged In: YES xxxpath.dif contains the splitext patch for posixpath, ntpath, dospath macpath and the corresponding test files (I have added a test file for macpath). I have found better to not attempt to modify riscospath.py since I don't know this platform. Anyway, it already use a rfind strategy. |
Logged In: YES The test_macpath module should probably use from test import test_support instead of import test_support Apart from this the patch looks fine. |
Logged In: YES Sebastien, thanks for the patch, and Armin, thanks for the macpath.py 1.41 (dospath has gone meanwhile) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: