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

Repr of collection's subclasses #71728

Closed
serhiy-storchaka opened this issue Jul 17, 2016 · 12 comments
Closed

Repr of collection's subclasses #71728

serhiy-storchaka opened this issue Jul 17, 2016 · 12 comments
Assignees
Labels
3.7 (EOL) end of life extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 27541
Nosy @rhettinger, @vstinner, @bitdancer, @serhiy-storchaka, @zhangyangyu
PRs
  • bpo-27541: Reprs of subclasses of some classes now contain actual type name. #3631
  • Dependencies
  • bpo-31497: Add _PyType_Name()
  • 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 = 'https://github.com/rhettinger'
    closed_at = <Date 2017-09-21.12:11:00.758>
    created_at = <Date 2016-07-17.08:54:36.677>
    labels = ['extension-modules', 'interpreter-core', 'type-feature', '3.7']
    title = "Repr of collection's subclasses"
    updated_at = <Date 2017-09-21.12:44:50.651>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-09-21.12:44:50.651>
    actor = 'vstinner'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2017-09-21.12:11:00.758>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules', 'Interpreter Core']
    creation = <Date 2016-07-17.08:54:36.677>
    creator = 'serhiy.storchaka'
    dependencies = ['31497']
    files = []
    hgrepos = []
    issue_num = 27541
    keywords = ['patch']
    message_count = 12.0
    messages = ['270621', '270623', '270625', '270627', '270646', '270650', '302387', '302684', '302687', '302689', '302690', '302691']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'vstinner', 'r.david.murray', 'serhiy.storchaka', 'xiang.zhang']
    pr_nums = ['3631']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue27541'
    versions = ['Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    The repr of subclasses of some collection classes contains a name of the subclass:

    >>> class S(set): pass
    ... 
    >>> S([1, 2, 3])
    S({1, 2, 3})
    >>> import collections
    >>> class OD(collections.OrderedDict): pass
    ... 
    >>> OD({1: 2})
    OD([(1, 2)])
    >>> class C(collections.Counter): pass
    ... 
    >>> C('senselessness')
    C({'s': 6, 'e': 4, 'n': 2, 'l': 1})

    But the repr of subclasses of some collection classes contains a name of the base class:

    >>> class BA(bytearray): pass
    ... 
    >>> BA([1, 2, 3])
    bytearray(b'\x01\x02\x03')
    >>> class D(collections.deque): pass
    ... 
    >>> D([1, 2, 3])
    deque([1, 2, 3])
    >>> class DD(collections.defaultdict): pass
    ... 
    >>> DD(int, {1: 2})
    defaultdict(<class 'int'>, {1: 2})

    Shouldn't a name of the subclass always be used?

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Jul 17, 2016
    @zhangyangyu
    Copy link
    Member

    How about other built-in classes? If repr does matter, maybe str, int, dict should also respect this rule?

    @serhiy-storchaka
    Copy link
    Member Author

    This can break third-party code. For example the code that explicitly makes the repr containing a subclass name:

    class MyStr(str):
        def __repr__(self):
            return 'MyStr(%s)' % str.__repr__(self)

    I think the chance of breaking third-party code for bytearray or deque is smaller, since the repr is not literal.

    @zhangyangyu
    Copy link
    Member

    Yes. So if we are not going to change other built-in types, maybe we'd better not change bytearray either. My opinion is that don't change built-in classes, even bytearray. If users would like a more reasonable repr, they can provide a custom __repr__ as your example. But make such changes to classes in collections sounds like a good idea to me.

    @rhettinger rhettinger self-assigned this Jul 17, 2016
    @bitdancer
    Copy link
    Member

    It certainly seems that collections should be consistent about this. The question of builtin types is a different issue, and I agree that it is probably way more trouble than it is worth to change them, especially since, for example, repr(str) is often used just to get the quote marks in contexts where you *don't* want the subclass name.

    @serhiy-storchaka
    Copy link
    Member Author

    It looks to me that the repr of a collection contains a dynamic name if it is implemented in Python and hardcoded base name if it is implemented in C (OrderedDict originally was implemented in Python). Maybe just because tp_name contains full qualified name, and extracting a bare class name needs few lines of code.

    There is similar issue with the io module classes: bpo-21861.

    Since this problem is already solved for OrderedDict, I think it is easy to use this solution in other classes. Maybe factoring out the following code into helper function.

        const char *classname;
        classname = strrchr(Py_TYPE(self)->tp_name, '.');
        if (classname == NULL)
            classname = Py_TYPE(self)->tp_name;
        else
            classname++;

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Sep 17, 2017
    @rhettinger
    Copy link
    Contributor

    +1 for changing the cases Serhiy found. Also, I agree with Serhiy that there should not be a change for the built-in types that have a literal notation (it has been this was forever, hasn't caused any real issues, and changing it now will likely break code).

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset b3a7796 by Serhiy Storchaka in branch 'master':
    bpo-27541: Reprs of subclasses of some classes now contain actual type name. (bpo-3631)
    b3a7796

    @serhiy-storchaka
    Copy link
    Member Author

    Thanks Raymond.

    @vstinner
    Copy link
    Member

    Why using type.__name__ rather than type.__qualname__?

    @serhiy-storchaka
    Copy link
    Member Author

    Because reprs of Python implementations of collection use a bare __name__.

    __qualname__ is used only in combination with __module__. Using a single __qualname__ can be confused: foo.bar looks as a name bar in the module foo. Whether in reprs and error messages either full qualified name is used ("{cls.__module__}.{qualname}") or a bare __name__. If a displayed name contains a dot it is always a full qualified name.

    @vstinner
    Copy link
    Member

    Because reprs of Python implementations of collection use a bare __name__.

    Ah, maybe this module should be updated to use qualified name with the name in repr()?

    __qualname__ is used only in combination with __module__.

    I was thinking at module.qualname, right.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants