gh-74453: Deprecate os.path.commonprefix

[sethmlarson]

Follow-up from #144401, this PR moves to deprecate os.path.commonprefix() in favor of the correctly behaving os.path.commonpath() and the behavior-describing string.commonprefix().

I'm not sure if this is all I have to do to deprecate an API, if there is more documentation that needs to happen let me know.

• Issue: Deprecate os.path.commonprefix #74453

---

📚 Documentation preview 📚: https://cpython-previews--144436.org.readthedocs.build/

[picnixz]

You need a What's New entry for that.

[picnixz]

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

[sethmlarson]

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.

[picnixz]

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.

[hugovk]

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.

[picnixz]

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.

[hugovk]

Please also list in Doc/deprecations/pending-removal-in-future.rst.

https://docs.python.org/dev/deprecations/index.html#pending-removal-in-future-versions

[hugovk]

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.

[sethmlarson]

Alright, most recent two commits should address all review comments. I have preserved backwards compatibility in the drop-in replacement string.commonprefix().

[picnixz]

I am sorry, but I do not want string.commonprefix to be equivalent to os.path.commonprefix. Semantically it doesn't make sense to have an os-related utility in string.

In addition, we only talk about "strings" in the documentation. Having an implicit os.fspath conversion should be documented otherwise. If we want to mimic os.path.commonpath in the first place, I do not see why we can't just keep it but soft-deprecate it.

Note that commonprefix does not care about the fact that we are strings. It also works for any sequence, as long as the elements can be ordered (e.g., we can use commonprefix for list of lists):

>>> os.path.commonpath([[1], [1,3]])
[1]

Since we expose it in string, anyone using a non-string as the first argument will not be able to make it work (because we raise a TypeError in os.fspath). So I would expect a string-like object to be acceptable but because of os.fspath it wouldn't be. This would be contrary to what the string module stands for.

If we want a commonprefix utility, I would suggest adding one to itertools, and use the latter. I believe it makes more sense to have something related to commonprefix out there. Or for strings only, have it in difflib.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

[sethmlarson]

Because I want to prioritize warning users about this function, I am okay with not providing a string.commonprefix() function and simply raising a DeprecationWarning in this PR. I want to avoid this warning getting delayed further, as has happened for the past ~30 years since the confusing behavior has been reported.

[sethmlarson]

@picnixz @hugovk I have removed string.commonprefix in 166c39d, I will happily revert or change this if there is a suggestion that should be completed in this PR.

(I have made the requested changes; please review again)

[sethmlarson]

I have made the requested changes; please review again

[bedevere-app]

Thanks for making the requested changes!

@hugovk, @picnixz: please review the changes made to this pull request.

[vstinner]

LGTM.

[vstinner]

Ah, this call is safe since it pass two lists of strings, and not two strings.

[hugovk]

Thanks!

[picnixz]

The question still remains about whether to expose string.commonprefix as something working on strings only (not paths), itertools.commonprefix (as working on any sequences) or the new seqtools module (I believe the discussion is stalled).

This would be a natural addition to that module, but that would also be a natural addition to more_itertools or another PyPI package dedicated to sequences utilities. OTOH, it'd still be possible to have it in itertools though supporting arbitrary iterables would make the implementation more complex.

OTOH, difflib could be used as an alternative (I mean, difflib seems to work for any sequence, except when we want to format the change but otherwise the diff algorithm looks generic?)

[vstinner]

The question still remains about whether to expose string.commonprefix as something working on strings only (not paths), itertools.commonprefix (as working on any sequences) or the new seqtools module (I believe the discussion is stalled).

I'm not sure that we have to provide a replacement function. os.path.commonpath() should cover most usages.

[hugovk]

Please open a new issue or discussion if you'd like a new method. We decided not to provide a replacement here, so it can be considered independently.

[picnixz]

I personally don't think it's necessary to provide a pure replacement but I don't know whether Seth wants to

[sethmlarson]

I don't feel that we need an exact replacement. string.commonprefix() only operating on strings seems fine, but if it's not an exact replacement then it can happen outside this PR since it's unrelated then.