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

BUG: Look up the _repr_pretty_ method on the class within the MRO rath... #2123

Merged
merged 1 commit into from Jul 14, 2012

Conversation

rkern
Copy link
Contributor

@rkern rkern commented Jul 11, 2012

...er than the original leaf class.

Thanks, @doerwalter for finding this and suggesting the fix.

fixes #2122

…ather than the original leaf class.

Thanks, @doerwalter for finding this and suggesting the fix.

fixes ipython#2122
@bfroehle bfroehle closed this Jul 11, 2012
@bfroehle bfroehle reopened this Jul 11, 2012
@bfroehle
Copy link
Contributor

Bah, didn't mean to accidentally close this PR. Reopening.

I'm a little curious why we don't just check for _repr_pretty_ on the actual object itself at the beginning of the method. I assume that it is so that the dispatch order can be more flexible:

  • Derived registered printer
  • Derived _repr_pretty_
  • Base registered printer
  • Base _repr_pretty_

However this does preclude adding _repr_pretty_ directly to the object as in

from __future__ import print_function

class A(object):
    _repr_pretty_ = None

a = A()
a._repr_pretty_ = lambda obj, p, cycle: print("A()")

All of this is surely outside the scope of the pull request (and original issue), but it is something to think about.

bfroehle added a commit that referenced this pull request Jul 14, 2012
BUG: Look up the `_repr_pretty_` method on the class using the MRO
@bfroehle bfroehle merged commit 4f10bd8 into ipython:master Jul 14, 2012
@bfroehle
Copy link
Contributor

Looks good, thanks for the tests, too!

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
BUG: Look up the `_repr_pretty_` method on the class using the MRO
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 this pull request may close these issues.

Bug in pretty printing
2 participants