[issue24912] The type of cached objects is mutable

[Nathaniel Smith]

Nathaniel Smith added the comment:

Guido van Rossum wrote:
> But first, why is it so important to assign the __class__ of a module?  It seems somebody is trying to make modules into what they weren't meant to be.

Because you told me to do it this way on python-dev :-(

https://mail.python.org/pipermail/python-dev/2014-December/137430.html

The goal is to make it possible for projects (e.g. numpy) to safely issue a deprecation warning when people access certain module-level constants.

The reason this is difficult is that we have several almost-conflicting requirements:

1) we want to be able to override special methods like __getattr__ and __dir__ on modules. And it'd nice if we could have access to things like properties and __repr__ too.

2) we can't control the type used to originally construct the module, because the module object is constructed before the first line of our code is run.

3) we want sys.modules[our_module].__dict__ to always refer to the namespace where __init__.py is executing, for reasons described at the top of msg249446. This is not currently possible when replacing the module object in-place -- if you make a new module object, then now you have the old module's namespace and the new module's namespace, and are responsible for manually keeping them in sync. (This is not a case where "let's do more of those" applies ;-).)

Other solutions that were previously considered (in two threads on python-dev and python-ideas with 40+ messages each) include:

- tackling (1) directly by defining a new set of special-purpose hooks just for modules (e.g. make ModuleType.__getattr__ check for a special __module_getattr__ function in the module namespace and call it). This is what Marc-Andre is suggesting now (msg249473). OTOH it would be nice to re-use the existing class mechanism instead of reimplementing parts of it just for modules.

- tackling (2) directly via wacky stuff like preparsing the file to check for special markers that tell the import machinery what ModuleType subclass to instantiate before the module starts executing (similar to how __future__ imports work)

- tackling (3) by adding some new special machinery to module objects, like the ability to replace their __dict__ attribute. This is what Eugene Toder is suggesting now (msg249483).

The advantage of allowing __class__ assignment on ModuleType instances is that it solves all these problems by using an existing feature rather than adding any new ones.

(I also tried the strategy of switching ModuleType to just *be* a heap type and avoid all these issues, but unfortunately it turns out that this would have broken the stable ABI so I gave up on that.)

The diff from 3.4 to 3.5rc2+the attached patch consists of uncontroversial bug fixes, plus two lines of code in typeobject.c that cause module subtypes to be treated similarly to heap types with respect to __class__ assignment:

-    if (!(newto->tp_flags & Py_TPFLAGS_HEAPTYPE) ||
-        !(oldto->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
+    if (!(PyType_IsSubtype(newto, &PyModule_Type) &&
+          PyType_IsSubtype(oldto, &PyModule_Type)) &&
+        (!(newto->tp_flags & Py_TPFLAGS_HEAPTYPE) ||
+         !(oldto->tp_flags & Py_TPFLAGS_HEAPTYPE))) {

These two lines of code solve an important user-facing issue for numpy, and are far simpler than any of the other proposed solutions. This approach was reviewed by python-dev, and has stood through the entire pre-release cycle so far without anyone producing a single argument yet for why it will cause any problem whatsoever.

I'm very sorry for introducing the bug with immutable types, and for not initially addressing it with the seriousness that it deserved. But that bug is fixed now, and unless someone can name an actual problem with the above two lines then I don't see why their association with a now-fixed bug is any reason to go rehash the entire discussion from scratch.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue24912>
_______________________________________