unittest.mock.patch('path') neither forwards or backwards compatible starting from Python >= v3.11 about cpython HOT 13 OPEN

bpo-44686/gh-88852 made the unittest.mock module use pkgutil.resolve_name() .

There is a subtle difference in the behavior. Both start with importing the first component of the dotted path, then resolve the following components relatively to the parent object, but

• pkgutil.resolve_name() first try to import the module with the {parent}.{name} name, and only if the import fails, it look up the name attribute in the parent module.
• The older unittest.mock code first tried to look up the name attribute in the parent module, and only if it was not found, it tried to import the module with the {parent}.{name} name.

It is difficult to say what way is more correct. The solution -- avoid ambiguous situations in which they give different results.

from cpython.

The issue is almost certainly unrelated to __all__; its only runtime effect is on from somemod import * which is not used here.

The issue is related to the fact that using from .Foo import Foo in an __init__.py is inherently bug prone, because it confuses the .Foo submodule with the Foo object within that submodule. Importing a submodule attaches an attribute onto the parent package which is the submodule, but you are also trying to attach an attribute on the parent package which is the Foo object from the submodule. This is a conflict.

Usually this kind of confusion is avoided in a case like this by sticking to the usual lowercase naming convention for modules, so you’d have from .apprise import Apprise instead, and non name collision. My recommendation is if possible you switch to this naming, or same kind of naming that avoids the name collision.

from cpython.

What is the Python's own import behavior?

If there is a package a with two submodules b and c, and a/__init__.py containing from . import c as b, then we get different results for resolving a.b.__name__, depending on the way how to import:

$ ./python -c 'import a.b; print(a.b.__name__)'
a.b
$ ./python -c 'from a import b; print(b.__name__)'
a.c
$ ./python -c 'from a.b import __name__; print(__name__)'
a.b

from cpython.

It may be not late to change the behavior if it is obviously wrong. The case is so obscure, that it affects not many people. But it is not obvious to me what variant is more right. I tried to change the behavior of pkgutil.resolve_name() -- and all tests are still passed.

It happens when a.b differs from sys.modules['a.b']. The workaround is to synchronize them: sys.modules['a.b'] = a.b. Perhaps.

from cpython.

In case of apprise, import apprise.Apprise as Apprise and from apprise import Apprise give different results. Actually, simple addition of import apprise.Apprise in the user code may ruin this package. I think this a sign of problems in such code.

from cpython.

#118176 adds tests for these corner cases.

from cpython.

I haven't closed this issue because I think it is worth understanding what actually changed in Python 3.11, so we can consider whether the new semantics are desirable, and if there's anything that should be better tested here to prevent future unintended changes.

Not sure if or when I'll have time to do that investigation.

from cpython.

@carljm I appreciate you getting back; thank you!

I cleaned up my demonstration repository and added python 3.7+ showing the breakage more. It's only for unittest.mock.patch() while direct import logic works perfectly in all versions of Python.

I now understand it has nothing to do with __all__ thanks to your explaination.

While I can understand your bug prone file naming reference, I can't see why the naming of a file should justify all out breakage in newer versions of Python and backwards compatibility loss (test cases only - so far). It's not like the files are prefixed with a period, contain utf-8, aren't syntaxtically correct, or lack a .py extension. All files can be successfully compiled into bytecode without problem as well. The can also all be seamlessly sourced correctly through import calls. This should be enough to justify a unique namespace reguardless of the filename?

The effort level in renaming all of my project (Apprise) files to lowercase will take some time (and testing) and will likely introduce breaking changes to those using it. I'll let you know if that helps 👍 .

I do appreciate you leaving the ticket open pending more investigation; I hope something gets uncovered by one who does have time to look into it further 🙏 .

from cpython.

@serhiy-storchaka awesome research!

But would breaking backwards compatibility not imply it was more correct before and the proposed change you described would be better handled during a Python v4 (breaking as it is)?

from cpython.

Hmm, I don't know the background of why pkgutil.resolve_name works the way it does, but it seems to me that pkgutil.resolve_name is buggy here, because its priority ordering of a package attribute vs submodule is backwards from that of Python's own import behavior. I don't know why this difference would be desirable.

from cpython.

Heh, good point :) I'd forgotten that Python itself is so inconsistent in such cases. This really suggests that "don't do this" is the only advisable course.

from cpython.

... "don't do this" is the only advisable course.

Personally it just demonstrates the issue is much larger. It's also apples to oranges in the sense that the way imports behave have no breaking changes between minor releases.

It also steps outside of the scope of the unittest which has a breaking change on it.Not then a 2-3 release depreciation phase or a release not to justify it (I'm hoping to be enlightened here 🙂🙏)?

from cpython.

I agree that the change in Python 3.11 was a breaking change, and I suspect that had that been recognized at the time, the change would not have been made.

But the breakage appears to have been rare in practice, since it's been 18 months since the release of Python 3.11, and this is the first it's been raised. And the breakage only impacts a usage that is both unusual and already inherently unreliable (thus already discouraged), given Python's inconsistency in whether a sub module or a package attribute is prioritized.

At this point there are already two Python feature releases out with the new behavior, and one of them is already into security-fix-only mode. So it is just too late for the "it was a back-compat break" argument to be relevant to what we do now. It was a back-compat break, and that's unfortunate. But we aren't going to compound that error by intentionally making another back-compat break now in 3.13, in order to revert to a behavior that doesn't appear to be inherently any better. And we certainly aren't going to backport a breaking change to Python 3.11 or 3.12, which are in security-fix-only and bug fix-only maintenance.

If the goal is to minimize breaking changes, the status quo is now the best option available.

from cpython.