__module__ attribute is not set correctly for a class created by direct metaclass call

@stevendaprano

BPO	28869
Nosy	@stevendaprano, @serhiy-storchaka, @ilevkivskyi, @Vgr255, @hongweipeng, @alegonz
PRs	gh-73055: Set class module to caller module in ABCMeta.__new__ #14126 gh-73055: Skip one additional frame when type.__new__ is called not directly from type.__call__. #14166 gh-73055: Set up inheritance registry in ABCMeta.__init__. #16613

^{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:

assignee = None
closed_at = None
created_at = <Date 2016-12-04.21:09:34.786>
labels = ['interpreter-core', '3.8', 'type-bug', '3.7', '3.9']
title = '__module__ attribute is not set correctly for a class created by direct metaclass call'
updated_at = <Date 2019-10-13.11:03:48.103>
user = 'https://github.com/ilevkivskyi'

bugs.python.org fields:

activity = <Date 2019-10-13.11:03:48.103>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2016-12-04.21:09:34.786>
creator = 'levkivskyi'
dependencies = []
files = []
hgrepos = []
issue_num = 28869
keywords = ['patch']
message_count = 20.0
messages = ['282363', '282365', '282367', '282369', '282378', '345073', '345226', '345328', '345332', '345770', '345772', '345797', '345889', '345945', '348210', '348234', '348312', '354071', '354263', '354573']
nosy_count = 6.0
nosy_names = ['steven.daprano', 'serhiy.storchaka', 'levkivskyi', 'abarry', 'hongweipeng', 'alegonz']
pr_nums = ['14126', '14166', '16613']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28869'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

__module__ attribute is set differently depending on whether a metaclass is explicitly called or invoked in a class statement:

>>> A = ABCMeta('A', (), {})
>>> A.__module__
'abc'
>>> class B(metaclass=ABCMeta): ...
... 
>>> B.__module__
'__main__'

Documentation on data model says that "__module__ is the module name in which the class was defined", so that the second behaviour seems right, while the first behaviour seems wrong to me.

As a matter of fact, A.__module__ in this case is abc.ABCMeta.__module__. A class body creates a __module__ key, while a direct metaclass call does not.

As a matter of fact, A.__module__ in this case is abc.ABCMeta.__module__. A class body creates a __module__ key, while a direct metaclass call does not.

But

>>> A = ABCMeta('A', (), {})
>>> ABCMeta.__module__ = 'hi'
>>> A.__module__
'abc'
>>> ABCMeta.__module__
'hi'

This means that the __module__ is copied from metaclass (also A.__dict__ actually contains '__module__' key, checked in 3.6).

Oh, nicely spotted! Apparently I was wrong, and it does create a key; defaulting to __name__.

About the original issue, I don't think it's easily possible to fix, sadly.

I had a brief look at the source for ABCMeta, and it seems to me that the module behaviour is coming from type. I'm not sure whether it can, or should, can be fixed in type, but I think that the correct behaviour for ABCMeta is to set module to the caller's global "name", not its own.

Something like this should probably work:

class ABCMeta(type):
    def __new__(mcls, name, bases, namespace):
        if '__module__' not in namespace:
            # globals()['__name__'] gives 'abc'
            frame = inspect.currentframe()
            if frame is not None:
                # IronPython? 
                caller_globals = frame.f_back.f_globals
                namespace['__module__'] = caller_globals['__name__']
        cls = super().__new__(mcls, name, bases, namespace)
        ...

Hello, I would like to add further on this issue.

(I'm new to bpo. Please advise if this report could be better)

Issue
-----

Pickling breaks when attempting to dump an instance of a class that was defined by calling ABCMeta directly, because it attempts to look for the class in abc rather than the module the class was defined in.

Below is a short snippet to reproduce the issue (my environment is Python 3.7.3 on Ubuntu 16.04)

>> import pickle
>> from abc import ABCMeta

>> class Foo(metaclass=ABCMeta):
>> pass

>> Bar = ABCMeta('Bar', (), {})

>> foo = Foo()
>> bar = Bar()

>> foo_p = pickle.dumps(foo) # OK
>> bar_p = pickle.dumps(bar) # PicklingError: Can't pickle <class 'abc.Bar'>: attribute lookup Bar on abc failed

I encountered this issue when working on a class factory to define classes dynamically. You can work around it if you smuggle {'__module__': __name__} when calling the metaclass, e.g.:

>> Qux = ABCMeta('Qux', (), {'__module__': __name__})
>> qux = Qux()
>> qux_p = pickle.dumps(qux) # OK

Apparently others have also stumbled upon this ABCMeta behavior before: https://stackoverflow.com/q/49457441

Some ideas to solve this
------------------------

A. Steven's proposal seems that could work (I haven't tested it myself yet though).
- This, however, would be limited to ABCMeta. Any other metaclass subclassed from type would have to include that workaround too for it to play well with pickle, which leads to,

B. Do A. and include in the documentation some recipe that shows this workaround when subclassing from type.

C. Implement A. in type itself. (Maybe around here? https://github.com/python/cpython/blob/master/Objects/typeobject.c#L2603)
- I'm not familiar with the internals of CPython, but I guess this would be the nuclear option.

If any of above ideas (or some other idea that could come up upon discussion) seems sensible, please allow me to work on it. I'd be happy to contribute :)

I think we can proceed with option A, but only if doesn't cause visible slow-down for creating ABCs (which is already slower that normal classes). TBH, I don't want to document A as "official" recipe (maybe however mention the problem in the pickle module docs).

Note that other "class factories" in stdlib (like collections.namedtuple) do almost exactly option A.

I think we can proceed with option A, but only if doesn't cause visible slow-down for creating ABCs (which is already slower that normal classes). TBH, I don't want to document A as "official" recipe (maybe however mention the problem in the pickle module docs).

I'll try using the same approach used in namedtuple, and see if there is any visible slow-down.

Regarding the documentation, I see your point. Perhaps adding some section like "Notes on pickling dynamically-defined classes" in the pickle module would be more appropriate? That section would mention that you have to make sure to set the __module__ attribute.

Note that other "class factories" in stdlib (like collections.namedtuple) do almost exactly option A.

Thanks for the tip. Indeed it does. In fact, the API defines a module argument. Its documentation could be improved, though. It should mention why would you want to pass that argument (pickling seems to be the reason that argument was added in the first place).

Perhaps adding some section like "Notes on pickling dynamically-defined classes" in the pickle module would be more appropriate?

I think just a note with few sentences would be enough.

Getting the module name from the caller's frame is an expensive operation. It is safe to set __module__ to None. In such case the pickle module is able to find the proper module containing the definition of the class.

>>> from abc import *
>>> A = ABCMeta('A', (), {})
>>> A.__module__
'abc'
>>> import pickle, pickletools
>>> pickletools.dis(pickletools.optimize(pickle.dumps(A)))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
_pickle.PicklingError: Can't pickle <class 'abc.A'>: attribute lookup A on abc failed
>>> A.__module__ = None
>>> pickletools.dis(pickletools.optimize(pickle.dumps(A)))
    0: \x80 PROTO      4
    2: \x95 FRAME      15
   11: \x8c SHORT_BINUNICODE '__main__'
   21: \x8c SHORT_BINUNICODE 'A'
   24: \x93 STACK_GLOBAL
   25: .    STOP
highest protocol among opcodes = 4

Getting the module name from the caller's frame is an expensive operation. It is safe to set __module__ to None. In such case the pickle module is able to find the proper module containing the definition of the class.

Wow, indeed it works. I also tried it from another module other than __main__ and it works.

Checking the source, I see the "magic" happens in pickle's whichmodule function when looping over sys.modules if __module__ is not found. If that case, why check for __module__ in the first place? wouldn't it be simpler to always loop over sys.modules? Is it to avoid looping over sys.modules whenever possible?

The problem is that __module__ is deduced from the caller's frame in type.__new__(). In case if type.__new__() is called directly, __module__ is set to the name of the module where type.__new__() is called. So virtually every type subclass which defines the __new__ method and calls type.__new__() inside it starves from the same issue.

It can be fixed in general if deduce __module__ in type.__call__() instead of type.__new__().

It can be fixed in general if deduce __module__ in type.__call__() instead of type.__new__().

But wouldn't we have the same problem if a metaclass overrides type.__call__ instead?
Also, wouldn't that reset the __module__ value anytime the class is called and an object is instantiated?

Hello,

Could anyone take a look at the PR 14126 submitted?
Sorry for the trouble and thank you in advance.

FWIW I like Serhiy's approach more. I have never seen a single metaclass overriding type.__call__, while overriding type.__new__ is a common practice.

I see, that’s an interesting point. In that case, Serhiy’s approach is indeed better.

PR 14166 seems stalled. I’d like to help if there’s anything left to do, if possible :)

There is a simpler way to fix this issue. Instead of hacking __new__, the inheritance registry can be set up in ABCMeta.__init__. There are no reasons of making this in __new__.

I think we can refer to typing.py, it does not have this issue.

>>> from typing import NamedTuple
>>> A = NamedTuple('A', [('name', str), ('id', int)])
>>> class B(NamedTuple):
...   name: str
...   id: int
... 
>>> A.__module__
'__main__'
>>> B.__module__
'__main__'

It uses nm_tpl.__module__ = sys._getframe(2).f_globals.get('__name__', '__main__').

PR 14166 now fixes the issue in more generic way. It skips one additional frame when type.__new__ is called not directly from type.__call__.

Navigation Menu

Repository navigation

module attribute is not set correctly for a class created by direct metaclass call #73055

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

__module__ attribute is not set correctly for a class created by direct metaclass call #73055

Description

Activity

ilevkivskyi commented on Dec 4, 2016

Vgr255 commented on Dec 4, 2016

ilevkivskyi commented on Dec 4, 2016

Vgr255 commented on Dec 4, 2016

stevendaprano commented on Dec 4, 2016

alegonz commented on Jun 9, 2019

ilevkivskyi commented on Jun 11, 2019

alegonz commented on Jun 12, 2019

ilevkivskyi commented on Jun 12, 2019

serhiy-storchaka commented on Jun 16, 2019

serhiy-storchaka commented on Jun 16, 2019

alegonz commented on Jun 17, 2019

serhiy-storchaka commented on Jun 17, 2019

alegonz commented on Jun 18, 2019

alegonz commented on Jul 20, 2019

ilevkivskyi commented on Jul 21, 2019

alegonz commented on Jul 23, 2019

serhiy-storchaka commented on Oct 7, 2019

hongweipeng commented on Oct 9, 2019

serhiy-storchaka commented on Oct 13, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

module attribute is not set correctly for a class created by direct metaclass call #73055