Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Doc/library/os.path.rst
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ the :mod:`glob` module.)
>>> os.path.commonpath(['/usr/lib', '/usr/local/lib'])
'/usr'

.. versionchanged:: 3.15
Comment thread
sethmlarson marked this conversation as resolved.
Outdated
Deprecated in favor of :func:`os.path.commonpath` for path prefixes and
:func:`string.commonprefix` for string prefixes.

.. versionchanged:: 3.6
Accepts a :term:`path-like object`.

Expand Down
10 changes: 10 additions & 0 deletions Doc/library/string.rst
Original file line number Diff line number Diff line change
Expand Up @@ -990,3 +990,13 @@ Helper functions
or ``None``, runs of whitespace characters are replaced by a single space
and leading and trailing whitespace are removed, otherwise *sep* is used to
split and join the words.

.. function:: commonprefix(list, /)

Return the longest string prefix (taken character-by-character) that is a
prefix of all string in *list*. If *list* is empty, return the empty string
Comment thread
sethmlarson marked this conversation as resolved.
Outdated
(``''``).

.. versionadded:: 3.15

Moved to the :mod:`string` module from the :mod:`os.path` module.
Comment thread
sethmlarson marked this conversation as resolved.
Outdated
20 changes: 7 additions & 13 deletions Lib/genericpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,13 @@ def getctime(filename, /):
# Return the longest prefix of all list elements.
def commonprefix(m, /):
"Given a list of pathnames, returns the longest common leading component"
if not m: return ''
# Some people pass in a list of pathname parts to operate in an OS-agnostic
# fashion; don't try to translate in that case as that's an abuse of the
# API and they are already doing what they need to be OS-agnostic and so
# they most likely won't be using an os.PathLike object in the sublists.
if not isinstance(m[0], (list, tuple)):
m = tuple(map(os.fspath, m))
s1 = min(m)
s2 = max(m)
for i, c in enumerate(s1):
if c != s2[i]:
return s1[:i]
return s1
import warnings
warnings.warn('os.path.commonprefix() is deprecated. Use '
'os.path.commonpath() or string.commonprefix() instead.',
Comment thread
sethmlarson marked this conversation as resolved.
Outdated
category=DeprecationWarning,
stacklevel=2)
import string
return string.commonprefix(m)

# Are two stat buffers (obtained from stat, fstat or lstat)
# describing the same file?
Expand Down
3 changes: 2 additions & 1 deletion Lib/posixpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import os
import sys
import stat
import string
import genericpath
from genericpath import *

Expand Down Expand Up @@ -542,7 +543,7 @@ def relpath(path, start=None):
start_list = start_tail.split(sep) if start_tail else []
path_list = path_tail.split(sep) if path_tail else []
# Work out how much of the filepath is shared by start and path.
i = len(commonprefix([start_list, path_list]))
i = len(string.commonprefix([start_list, path_list]))

rel_list = [pardir] * (len(start_list)-i) + path_list[i:]
if not rel_list:
Expand Down
24 changes: 22 additions & 2 deletions Lib/string/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
"""

__all__ = ["ascii_letters", "ascii_lowercase", "ascii_uppercase", "capwords",
"digits", "hexdigits", "octdigits", "printable", "punctuation",
"whitespace", "Formatter", "Template"]
"commonprefix", "digits", "hexdigits", "octdigits", "printable",
"punctuation", "whitespace", "Formatter", "Template"]

import _string

Expand Down Expand Up @@ -48,6 +48,26 @@ def capwords(s, sep=None):
return (sep or ' ').join(map(str.capitalize, s.split(sep)))


def commonprefix(m, /):
"Given a list of strings, returns the longest common leading component"
if not m: return ''
Comment thread
sethmlarson marked this conversation as resolved.
Outdated
# Note that previously this function was in the 'os.path' module, hence the
# handling for paths. Maintain compatibility so users have a 1-to-1 drop-in.
# Some people pass in a list of pathname parts to operate in an OS-agnostic
# fashion; don't try to translate in that case as that's an abuse of the
# API and they are already doing what they need to be OS-agnostic and so
# they most likely won't be using an os.PathLike object in the sublists.
if not isinstance(m[0], (list, tuple)):
import os
m = tuple(map(os.fspath, m))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really fond of this. string.commonprefix() should be specific to strings and should not care about pathlike objects.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is only done for backwards compatibility. I would be happy to get rid of it, but I figured that this would be not allowed considering we're doing a "rename", not a new API.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no backwards compatibility here. We create a new function in a module that is not related to filepaths at all. I do not consider this as a rename, I really consider this as a new API. Instead, I would keep the implementation of os.path.commonpath as is and add in a follow-up PR string.commonprefix which does not do the fspath computation. We then document that os.path.commonpath should be reimplemented as string.commonprefix(tuple(map(os.fspath, m))) if needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this new string.commonprefix() should be a direct, one-to-one, drop-in replacement for the old os.path.commonprefix().

We're not doing anyone favours by changing the behaviour and making them [have to decide whether they need to] jump through extra hoops.

Chances are they should be using os.path.commonpath() instead, but if not, let them just replace the module name and keep the same old behaviour.

I doubt there's significant performance improvement from changing it either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me this isn't about performance, it's about semantics. string contains string-related utilities, not path-related ones. I am really opposed to having string.commonprefix being smart here. I know that we are not necessarily doing the user a favor but I don't want to have unrelated utilities in string. It is, for me, the wrong module for that.

s1 = min(m)
s2 = max(m)
for i, c in enumerate(s1):
if c != s2[i]:
return s1[:i]
return s1


####################################################################
_sentinel_dict = {}

Expand Down
136 changes: 69 additions & 67 deletions Lib/test/test_genericpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,71 +34,72 @@ def test_no_argument(self):
.format(self.pathmodule.__name__, attr))

def test_commonprefix(self):
commonprefix = self.pathmodule.commonprefix
self.assertEqual(
commonprefix([]),
""
)
self.assertEqual(
commonprefix(["/home/swenson/spam", "/home/swen/spam"]),
"/home/swen"
)
self.assertEqual(
commonprefix(["/home/swen/spam", "/home/swen/eggs"]),
"/home/swen/"
)
self.assertEqual(
commonprefix(["/home/swen/spam", "/home/swen/spam"]),
"/home/swen/spam"
)
self.assertEqual(
commonprefix(["home:swenson:spam", "home:swen:spam"]),
"home:swen"
)
self.assertEqual(
commonprefix([":home:swen:spam", ":home:swen:eggs"]),
":home:swen:"
)
self.assertEqual(
commonprefix([":home:swen:spam", ":home:swen:spam"]),
":home:swen:spam"
)

self.assertEqual(
commonprefix([b"/home/swenson/spam", b"/home/swen/spam"]),
b"/home/swen"
)
self.assertEqual(
commonprefix([b"/home/swen/spam", b"/home/swen/eggs"]),
b"/home/swen/"
)
self.assertEqual(
commonprefix([b"/home/swen/spam", b"/home/swen/spam"]),
b"/home/swen/spam"
)
self.assertEqual(
commonprefix([b"home:swenson:spam", b"home:swen:spam"]),
b"home:swen"
)
self.assertEqual(
commonprefix([b":home:swen:spam", b":home:swen:eggs"]),
b":home:swen:"
)
self.assertEqual(
commonprefix([b":home:swen:spam", b":home:swen:spam"]),
b":home:swen:spam"
)

testlist = ['', 'abc', 'Xbcd', 'Xb', 'XY', 'abcd',
'aXc', 'abd', 'ab', 'aX', 'abcX']
for s1 in testlist:
for s2 in testlist:
p = commonprefix([s1, s2])
self.assertStartsWith(s1, p)
self.assertStartsWith(s2, p)
if s1 != s2:
n = len(p)
self.assertNotEqual(s1[n:n+1], s2[n:n+1])
with warnings_helper.check_warnings((".*commonpath().*", DeprecationWarning)):
commonprefix = self.pathmodule.commonprefix
Comment thread
sethmlarson marked this conversation as resolved.
Outdated
self.assertEqual(
commonprefix([]),
""
)
self.assertEqual(
commonprefix(["/home/swenson/spam", "/home/swen/spam"]),
"/home/swen"
)
self.assertEqual(
commonprefix(["/home/swen/spam", "/home/swen/eggs"]),
"/home/swen/"
)
self.assertEqual(
commonprefix(["/home/swen/spam", "/home/swen/spam"]),
"/home/swen/spam"
)
self.assertEqual(
commonprefix(["home:swenson:spam", "home:swen:spam"]),
"home:swen"
)
self.assertEqual(
commonprefix([":home:swen:spam", ":home:swen:eggs"]),
":home:swen:"
)
self.assertEqual(
commonprefix([":home:swen:spam", ":home:swen:spam"]),
":home:swen:spam"
)

self.assertEqual(
commonprefix([b"/home/swenson/spam", b"/home/swen/spam"]),
b"/home/swen"
)
self.assertEqual(
commonprefix([b"/home/swen/spam", b"/home/swen/eggs"]),
b"/home/swen/"
)
self.assertEqual(
commonprefix([b"/home/swen/spam", b"/home/swen/spam"]),
b"/home/swen/spam"
)
self.assertEqual(
commonprefix([b"home:swenson:spam", b"home:swen:spam"]),
b"home:swen"
)
self.assertEqual(
commonprefix([b":home:swen:spam", b":home:swen:eggs"]),
b":home:swen:"
)
self.assertEqual(
commonprefix([b":home:swen:spam", b":home:swen:spam"]),
b":home:swen:spam"
)

testlist = ['', 'abc', 'Xbcd', 'Xb', 'XY', 'abcd',
'aXc', 'abd', 'ab', 'aX', 'abcX']
for s1 in testlist:
for s2 in testlist:
p = commonprefix([s1, s2])
self.assertStartsWith(s1, p)
self.assertStartsWith(s2, p)
if s1 != s2:
n = len(p)
self.assertNotEqual(s1[n:n+1], s2[n:n+1])

def test_getsize(self):
filename = os_helper.TESTFN
Expand Down Expand Up @@ -606,8 +607,9 @@ def test_path_isdir(self):
self.assertPathEqual(os.path.isdir)

def test_path_commonprefix(self):
self.assertEqual(os.path.commonprefix([self.file_path, self.file_name]),
self.file_name)
with warnings_helper.check_warnings((".*commonpath().*", DeprecationWarning)):
self.assertEqual(os.path.commonprefix([self.file_path, self.file_name]),
self.file_name)

def test_path_getsize(self):
self.assertPathEqual(os.path.getsize)
Expand Down
14 changes: 8 additions & 6 deletions Lib/test/test_ntpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from ntpath import ALL_BUT_LAST, ALLOW_MISSING
from test import support
from test.support import os_helper
from test.support import warnings_helper
from test.support.os_helper import FakePath
from test import test_genericpath
from tempfile import TemporaryFile
Expand Down Expand Up @@ -298,12 +299,13 @@ def test_isabs(self):
tester('ntpath.isabs("\\\\.\\C:")', 1)

def test_commonprefix(self):
Comment thread
sethmlarson marked this conversation as resolved.
tester('ntpath.commonprefix(["/home/swenson/spam", "/home/swen/spam"])',
"/home/swen")
tester('ntpath.commonprefix(["\\home\\swen\\spam", "\\home\\swen\\eggs"])',
"\\home\\swen\\")
tester('ntpath.commonprefix(["/home/swen/spam", "/home/swen/spam"])',
"/home/swen/spam")
with warnings_helper.check_warnings((".*commonpath().*", DeprecationWarning)):
tester('ntpath.commonprefix(["/home/swenson/spam", "/home/swen/spam"])',
"/home/swen")
tester('ntpath.commonprefix(["\\home\\swen\\spam", "\\home\\swen\\eggs"])',
"\\home\\swen\\")
tester('ntpath.commonprefix(["/home/swen/spam", "/home/swen/spam"])',
"/home/swen/spam")

def test_join(self):
tester('ntpath.join("")', '')
Expand Down
92 changes: 92 additions & 0 deletions Lib/test/test_string/test_string.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from string import Template
import types
from test.support import cpython_only
from test.support import os_helper
from test.support.import_helper import ensure_lazy_imports


Expand Down Expand Up @@ -40,6 +41,97 @@ def test_capwords(self):
self.assertEqual(string.capwords('\taBc\tDeF\t'), 'Abc Def')
self.assertEqual(string.capwords('\taBc\tDeF\t', '\t'), '\tAbc\tDef\t')

def test_commonprefix(self):
self.assertEqual(
string.commonprefix([]),
""
)
self.assertEqual(
string.commonprefix(["a", "b"]),
""
)
self.assertEqual(
string.commonprefix(["/home/swenson/spam", "/home/swen/spam"]),
"/home/swen"
)
self.assertEqual(
string.commonprefix(["/home/swen/spam", "/home/swen/eggs"]),
"/home/swen/"
)
self.assertEqual(
string.commonprefix(["/home/swen/spam", "/home/swen/spam"]),
"/home/swen/spam"
)
self.assertEqual(
string.commonprefix(["home:swenson:spam", "home:swen:spam"]),
"home:swen"
)
self.assertEqual(
string.commonprefix([":home:swen:spam", ":home:swen:eggs"]),
":home:swen:"
)
self.assertEqual(
string.commonprefix([":home:swen:spam", ":home:swen:spam"]),
":home:swen:spam"
)

self.assertEqual(
string.commonprefix([b"/home/swenson/spam", b"/home/swen/spam"]),
b"/home/swen"
)
self.assertEqual(
string.commonprefix([b"/home/swen/spam", b"/home/swen/eggs"]),
b"/home/swen/"
)
self.assertEqual(
string.commonprefix([b"/home/swen/spam", b"/home/swen/spam"]),
b"/home/swen/spam"
)
self.assertEqual(
string.commonprefix([b"home:swenson:spam", b"home:swen:spam"]),
b"home:swen"
)
self.assertEqual(
string.commonprefix([b":home:swen:spam", b":home:swen:eggs"]),
b":home:swen:"
)
self.assertEqual(
string.commonprefix([b":home:swen:spam", b":home:swen:spam"]),
b":home:swen:spam"
)

testlist = ['', 'abc', 'Xbcd', 'Xb', 'XY', 'abcd',
'aXc', 'abd', 'ab', 'aX', 'abcX']
for s1 in testlist:
for s2 in testlist:
p = string.commonprefix([s1, s2])
self.assertStartsWith(s1, p)
self.assertStartsWith(s2, p)
if s1 != s2:
n = len(p)
self.assertNotEqual(s1[n:n+1], s2[n:n+1])

def test_commonprefix_paths(self):
# Test backwards-compatibility with os.path.commonprefix()
# This function must handle PathLike objects.
file_name = os_helper.TESTFN
file_path = os_helper.FakePath(file_name)
self.assertEqual(string.commonprefix([file_path, file_name]),
file_name)

def test_commonprefix_sequence_of_str(self):
# Test backwards-compatibility with os.path.commonprefix()
# This function must handle lists and tuples of strings.
for type_ in (tuple, list):
seq1 = type_(["abc", "de", "fgh"])
seq2 = type_(["abc", "def", "gh"])
self.assertEqual(string.commonprefix([seq1, seq2]),
type_(["abc"]))

seq1 = type_(["ab"])
seq2 = type_(["ac"])
self.assertEqual(string.commonprefix([seq1, seq2]), type_([]))

def test_basic_formatter(self):
fmt = string.Formatter()
self.assertEqual(fmt.format("foo"), "foo")
Expand Down
2 changes: 1 addition & 1 deletion Lib/textwrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ def dedent(text):
msg = f'expected str object, not {type(text).__qualname__!r}'
raise TypeError(msg) from None

# Get length of leading whitespace, inspired by ``os.path.commonprefix()``.
# Get length of leading whitespace, inspired by ``string.commonprefix()``.
non_blank_lines = [l for l in lines if l and not l.isspace()]
l1 = min(non_blank_lines, default='')
l2 = max(non_blank_lines, default='')
Expand Down
Loading
Loading