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

Show the address in the repr for class objects #69734

Closed
rhettinger opened this issue Nov 3, 2015 · 27 comments
Closed

Show the address in the repr for class objects #69734

rhettinger opened this issue Nov 3, 2015 · 27 comments
Assignees
Labels
release-blocker type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 25548
Nosy @gvanrossum, @brettcannon, @rhettinger, @terryjreedy, @ned-deily, @petere, @vadmium, @kushaldas, @matrixise, @pppery, @timgraham, @Carreau, @Vgr255, @Shredder13
Files
  • issue25548.patch: Code and test updates for the same
  • issue25548v2.patch: Second version of the patch
  • issue25548v4.patch: with news update
  • 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/kushaldas'
    closed_at = <Date 2016-07-14.04:13:42.855>
    created_at = <Date 2015-11-03.18:40:11.560>
    labels = ['type-feature', 'release-blocker']
    title = 'Show the address in the repr for class objects'
    updated_at = <Date 2016-07-14.04:13:42.849>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2016-07-14.04:13:42.849>
    actor = 'python-dev'
    assignee = 'kushal.das'
    closed = True
    closed_date = <Date 2016-07-14.04:13:42.855>
    closer = 'python-dev'
    components = []
    creation = <Date 2015-11-03.18:40:11.560>
    creator = 'rhettinger'
    dependencies = []
    files = ['43119', '43151', '43153']
    hgrepos = []
    issue_num = 25548
    keywords = ['patch']
    message_count = 27.0
    messages = ['254012', '254017', '254232', '265883', '266038', '266963', '267110', '267112', '267113', '267119', '267121', '267122', '267225', '267286', '267344', '267403', '267704', '268760', '268763', '269075', '270262', '270296', '270313', '270314', '270342', '270345', '270360']
    nosy_count = 16.0
    nosy_names = ['gvanrossum', 'brett.cannon', 'rhettinger', 'terry.reedy', 'ned.deily', 'petere', 'python-dev', 'martin.panter', 'kushal.das', 'matrixise', 'ppperry', 'Tim.Graham', 'mbussonn', 'abarry', 'Winterflower', 'Nofar Schnider']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue25548'
    versions = ['Python 3.6']

    @rhettinger
    Copy link
    Contributor Author

    In old-style classes, the memory address of the class object is shown for both the class and for the instance. In new-style classes, only the instance shows the address. I and other Python instructors have found the memory address display to be useful and I think it should be added back:

    Old-style
    =========

     
      >>> class A:
      	pass
    
      >>> a = A()
      >>> A
      <class __main__.A at 0x10061e328>
      >>> a
      <__main__.A instance at 0x105292cb0> 

    New-style
    =========

      >>> class A(object):
      	pass
    
      >>> a = A()
      >>> A
      <class '__main__.A'>
      >>> a
      <__main__.A object at 0x105332cd0>

    @rhettinger rhettinger added the type-feature A feature request or enhancement label Nov 3, 2015
    @matrixise
    Copy link
    Member

    Hi Raymond,

    I just executed the code with python 3.5 and I don't have this result:

    Python 3.5.0 (default, Sep 23 2015, 04:41:38)
    [GCC 4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.0.72)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> class A: pass
    ...
    >>> A
    <class '__main__.A'>
    >>> class A(object): pass
    ...
    >>> A
    <class '__main__.A'>
    >>>

    @terryjreedy
    Copy link
    Member

    In 3.x, old style classes are gone, so you get a new style class with or without '(object)'.

    @pppery
    Copy link
    Mannequin

    pppery mannequin commented May 19, 2016

    I don't quite get why the memory address is helpful, but you could work around this using a custom metaclass:

        class IdMeta(type):
            def __repr__(cls):
                return super().__repr__()[:-1] + " at 0x%x>"%id(cls)
        class IdInRepr(metaclass=IdMeta):pass

    IdInRepr and all its subclasses will then have the memory address in their representation:

    <class 'module.name' at address>

    @rhettinger
    Copy link
    Contributor Author

    I don't quite get why the memory address is helpful

    For the same reason that we've found the address to helpful in other reprs, it helps people understand that classes are objects just like anything else and to know which objects are distinct.

    My instructors have found that this matters when teaching Python and more than one of them immediately noticed the loss of information when switching from teaching Python 2 to Python 3.

    @kushaldas
    Copy link
    Member

    Attaching the patch for the same. Had to update the test cases for the following tests to have this behavior as expected.

    test_functools
    test_cmd_line_script
    test_ctypes
    test_defaultdict
    test_descr
    test_descrtut
    test_doctest
    test_generators
    test_genexps
    test_metaclass
    test_pprint
    test_reprlib
    test_statistics
    test_trace
    test_wsgiref

    @kushaldas
    Copy link
    Member

    Uploading the new patch with a new test case for the same.

    @Vgr255
    Copy link
    Mannequin

    Vgr255 mannequin commented Jun 3, 2016

    I'd probably change all instances of ".*" in the regex matches to be "0x.+" instead. For the docstrings that have "..." in them, I would probably make those " at ..." as well (although you get decide if it hinders readability too much).

    Other than that patch LGTM.

    @brettcannon
    Copy link
    Member

    Patch LGTM. Just needs doc and What's New updates.

    @rhettinger
    Copy link
    Contributor Author

    Kushal. If you don't mind, I would like Nofar to be able to do this patch. She's been working on it since before the sprints and was slowed down by a schoolwork crunch.

    @rhettinger
    Copy link
    Contributor Author

    Nofar is about to upload her patch as well (she's been working on this for a while). Perhaps the two can be compared and reconciled so that both can be credited on the commit.

    @kushaldas
    Copy link
    Member

    Hey Raymond, I am uploading the patch which I almost committed along with whats new update :)
    I am also reassigning the ticket to you so that you can decide the next steps.

    @kushaldas kushaldas assigned rhettinger and unassigned kushaldas Jun 3, 2016
    @rhettinger
    Copy link
    Contributor Author

    Thanks Kushal. It looks like we had a race condition ;-) I'll assign it back after Nofar's work is in and reconciled.

    @rhettinger
    Copy link
    Contributor Author

    Nofar said not to wait for her on this one.

    @rhettinger rhettinger assigned kushaldas and unassigned rhettinger Jun 4, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 4, 2016

    New changeset af29d89083b3 by Kushal Das in branch 'default':
    Issue bpo-25548: Showing memory address of class objects in repl
    https://hg.python.org/cpython/rev/af29d89083b3

    @Shredder13
    Copy link
    Mannequin

    Shredder13 mannequin commented Jun 5, 2016

    Kushal, you've beat me to it. Great work!

    @Carreau
    Copy link
    Mannequin

    Carreau mannequin commented Jun 7, 2016

    Hi guys, the title of the issues is "show address in the **repR**", but the NEWS file says[1] in the **repL*, which are 2 different things ! :-)

    And this patch change the repR so it also affect scripts and unittests.

    [1] since https://hg.python.org/cpython/rev/af29d89083b3

    @petere
    Copy link
    Mannequin

    petere mannequin commented Jun 18, 2016

    I understand the reasoning here, but I want to say booh to this change in 3.6.0a2 because it breaks my tests. It used to be that type(x) returned a predictable string, and that was an easy way to verify the result types of things.

    Perhaps a __str__ implementation could be added that avoids the memory address?

    @vadmium
    Copy link
    Member

    vadmium commented Jun 18, 2016

    There is also bpo-13224 proposing to change __str__() to just return the __name__ or __qualname__ if I remember correctly.

    @timgraham
    Copy link
    Mannequin

    timgraham mannequin commented Jun 22, 2016

    I'll echo what Peter said and say that this breaks 5 tests in Django's test suite which are checking error messages. If it stays, perhaps it could be added to the release notes instead of just NEWS.

    @gvanrossum
    Copy link
    Member

    I'm also echoing this... It breaks too many tests. I filed bpo-27498.

    @kushaldas
    Copy link
    Member

    The NEWS file got a typo. Thanks for noticing that.
    This change did require a lot of updates to the tests.
    If rest of the people agrees to revert this change, then we should go ahead and do it. Just wondering if Raymond has anything to add.

    @rhettinger
    Copy link
    Contributor Author

    Would there be a way to confine this to just heap types? The user defined classes are where there is the most benefit. For the built-in types, it just seems to add noise.

    @gvanrossum
    Copy link
    Member

    Doing it only for user-defined types would still break my tests.

    @rhettinger
    Copy link
    Contributor Author

    +1 on rolling this back. It sounds like it hurt more than it would have helped.

    To satisfy my curiosity, can you post one of the tests that broke. It would be nice to see what kind of tests are depend the repr of the class. AFAICT, there was never an issue with this for old-style classes in Python2.7, so something new must be occurring in the wild.

    @gvanrossum
    Copy link
    Member

    Thanks, let's roll it back.

    The reason it never was an issue for old-style classes is that they
    behaved like this from the start, so nobody wrote tests that depended
    on the predictability of repr(). But new-style classes have had this
    nice clean repr() since they were introduced (in 2.3?) so it's
    unsurprising that this is now depended upon.

    Here's a link to some test code for mypy that broke:
    https://github.com/python/mypy/blob/master/mypy/util.py#L22-L23

    It may be irreprehensible code but it works for Python 3.2-3.5 (and
    for new-style classes in Python 2, except mypy requires Python 3), and
    broke in 3.6.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 14, 2016

    New changeset 4f11e6b72e8f by Benjamin Peterson in branch 'default':
    Backed out changeset af29d89083b3 (closes bpo-25548) (closes bpo-27498)
    https://hg.python.org/cpython/rev/4f11e6b72e8f

    @python-dev python-dev mannequin closed this as completed Jul 14, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    release-blocker type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants