Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subclassing NamedDict leads to infinite recursion in __repr__ #39

Closed
jmaroeder opened this issue Nov 9, 2017 · 4 comments · Fixed by #44
Closed

Subclassing NamedDict leads to infinite recursion in __repr__ #39

jmaroeder opened this issue Nov 9, 2017 · 4 comments · Fixed by #44

Comments

@jmaroeder
Copy link
Contributor

Attempting to subclass NamedDict leads to an infinite recursion error when attempting to print out the repr of an instance of the class. This is due to the use of type(self) in the super call.

The docs address this (http://maps.readthedocs.io/en/latest/user/design.html#class-vs-function), but I think there is a use case for being able to subclass NamedDict - specifically, if you want to be able to distinguish between types of NamedDict using isinstance, and/or if you want to add specific functionality to a bag of data.

NamedDict is a trivial fix - just change the occurrences of super(type(self), self) to super(NamedDict, self).

However, the metaclasses NamedFrozenMapMeta and NamedFixedKeyMapMeta still contain occurrences of super(type(self), self). Due to the way the subclasses are created (using exec), it's not immediately apparent to me how one would address this issue - there seems to be a chicken and egg problem in we need to pass a reference to the class into the namespace, but we don't have that reference until it's created. I tried attaching __init__ to the class after creation via type.__new__, but that led to other issues:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 2, in __init__
  File "/Users/jamesallen/src/github.com/jamesmallen/maps/maps/frozenmap.py", line 28, in __init__
    self._data = dict(*args, **kwargs)
  File "/Users/jamesallen/src/github.com/jamesmallen/maps/maps/namedfrozenmap.py", line 66, in _setattr
    super(NamedFrozenMapMeta, self).__setattr__(name, value)
TypeError: super(type, obj): obj must be an instance or subtype of type
@pcattori
Copy link
Owner

Thanks @jamesmallen . I'll look into this later this week as well.

@pcattori
Copy link
Owner

unpolished thought:

one way to resolve this would be to have functions responsible for creating the injected methods and these functions could accept the typename as an argument:

# namedfixedkeymap.py

...

@staticmethod
def _make_repr(typename):
    def _repr(self): # pragma: no cover
        kwargs = ', '.join('{}={!r}'.format(key, value) for key, value in self.items())
        return '{}({})'.format(typename, kwargs)
    return _repr

...

methods = {
    ...
    '__repr__': NamedFixedKeyMapMeta._make_repr(typename),
    ...

@pcattori
Copy link
Owner

pcattori commented Nov 29, 2017

On second thought, I believe the only reason an infinite recursion occurs is because __repr__ is calling itself via super(...).__repr__(), so I would think the problem is contained to just NamedDict.

Aside: type(self) is actually desired in most cases so that subclasses don't have to reimplement methods just to hardcode their name everywhere

@jamesmallen have you witnessed any infinite recursion with maps other than NamedDict (or ideally an example that currently fails)?

@pcattori pcattori mentioned this issue Nov 29, 2017
@pcattori
Copy link
Owner

@jamesmallen feel free to reopen if you do have other cases beyond NamedDict where this infinite recursion happens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants