Repository: ipython/ipython
Pull request: fixed honoring custom repr for NamedTuple if assigned by partialmethod
Fixes #14465.

- added test `test_custom_repr_namedtuple_partialmethod`

Problem statement:
`display` does not honor custom `__repr__` for `NamedTuple` when defined by `partialmethod`
The issue occurs when the custom `__repr__` is defined via `partialmethod`:

```python
from functools import partialmethod
from typing import NamedTuple

def my_pprint(obj, **options):
    return "Hello World"

def add_custom_pprint(cls=None, **options):
    if cls is None:
        def decorator(cls):
            return add_custom_pprint(cls, **options)
        return decorator
    cls.__repr__ = partialmethod(my_pprint, **options)
    return cls

@add_custom_pprint
class MyNamedTuple(NamedTuple):
    x: int
    y: int

x = MyNamedTuple(1, 2)
print(x)  # "Hello World"
display(x)  # (1, 2)
```

<details><summary> without partialmethod, both display and print give the same result: </summary>

```python
from typing import NamedTuple

def my_pprint(obj, **options):
    return "Hello World"

def add_custom_pprint(cls=None, **options):
    if cls is None:
        def decorator(cls):
            return add_custom_pprint(cls, **options)
        return decorator
    cls.__repr__ = my_pprint
    return cls

@add_custom_pprint
class MyNamedTuple(NamedTuple):
    x: int
    y: int

x = MyNamedTuple(1, 2)
print(x)  # "Hello World"
display(x)   # "Hello World"
```

</details>

Hints from issue discussion:
The culprit:

https://github.com/ipython/ipython/blob/1454013b8b615ce75d30d38b7710e3fccca16263/IPython/lib/pretty.py#L409-L411

This check is incorrect, because `partialmethod` is a descriptor and not callable. Instead, the test should be replaced with `callable(getattr(cls, "__repr__", None))`.

Commit message: formatting

Patch to review:
diff --git a/IPython/core/tests/test_formatters.py b/IPython/core/tests/test_formatters.py
index c642befaccf..cd8c83e555b 100644
--- a/IPython/core/tests/test_formatters.py
+++ b/IPython/core/tests/test_formatters.py
@@ -103,7 +103,7 @@ def set_fp(p):
 
 def test_for_type():
     f = PlainTextFormatter()
-    
+
     # initial return, None
     assert f.for_type(C, foo_printer) is None
     # no func queries
@@ -116,9 +116,9 @@ def test_for_type():
 
 def test_for_type_string():
     f = PlainTextFormatter()
-    
+
     type_str = '%s.%s' % (C.__module__, 'C')
-    
+
     # initial return, None
     assert f.for_type(type_str, foo_printer) is None
     # no func queries
@@ -130,9 +130,9 @@ def test_for_type_string():
 
 def test_for_type_by_name():
     f = PlainTextFormatter()
-    
+
     mod = C.__module__
-    
+
     # initial return, None
     assert f.for_type_by_name(mod, "C", foo_printer) is None
     # no func queries
@@ -146,7 +146,7 @@ def test_for_type_by_name():
 
 def test_lookup():
     f = PlainTextFormatter()
-    
+
     f.for_type(C, foo_printer)
     assert f.lookup(C()) is foo_printer
     with pytest.raises(KeyError):
@@ -155,7 +155,7 @@ def test_lookup():
 def test_lookup_string():
     f = PlainTextFormatter()
     type_str = '%s.%s' % (C.__module__, 'C')
-    
+
     f.for_type(type_str, foo_printer)
     assert f.lookup(C()) is foo_printer
     # should move from deferred to imported dict
@@ -173,16 +173,16 @@ def test_lookup_by_type_string():
     f = PlainTextFormatter()
     type_str = '%s.%s' % (C.__module__, 'C')
     f.for_type(type_str, foo_printer)
-    
+
     # verify insertion
     assert _mod_name_key(C) in f.deferred_printers
     assert C not in f.type_printers
-    
+
     assert f.lookup_by_type(type_str) is foo_printer
     # lookup by string doesn't cause import
     assert _mod_name_key(C) in f.deferred_printers
     assert C not in f.type_printers
-    
+
     assert f.lookup_by_type(C) is foo_printer
     # should move from deferred to imported dict
     assert _mod_name_key(C) not in f.deferred_printers
@@ -220,10 +220,10 @@ def test_pop():
 def test_pop_string():
     f = PlainTextFormatter()
     type_str = '%s.%s' % (C.__module__, 'C')
-    
+
     with pytest.raises(KeyError):
         f.pop(type_str)
-    
+
     f.for_type(type_str, foo_printer)
     f.pop(type_str)
     with pytest.raises(KeyError):
@@ -238,7 +238,7 @@ def test_pop_string():
     with pytest.raises(KeyError):
         f.pop(type_str)
     assert f.pop(type_str, None) is None
-    
+
 
 def test_error_method():
     f = HTMLFormatter()
@@ -341,14 +341,14 @@ def __getattr__(self, key):
     assert text_hat._repr_html_ == "_repr_html_"
     with capture_output() as captured:
         result = f(text_hat)
-    
+
     assert result is None
     assert "FormatterWarning" not in captured.stderr
 
     class CallableMagicHat(object):
         def __getattr__(self, key):
             return lambda : key
-    
+
     call_hat = CallableMagicHat()
     with capture_output() as captured:
         result = f(call_hat)
@@ -358,11 +358,11 @@ def __getattr__(self, key):
     class BadReprArgs(object):
         def _repr_html_(self, extra, args):
             return "html"
-    
+
     bad = BadReprArgs()
     with capture_output() as captured:
         result = f(bad)
-    
+
     assert result is None
     assert "FormatterWarning" not in captured.stderr
 
@@ -406,13 +406,13 @@ def _ipython_display_(self):
     class NotSelfDisplaying(object):
         def __repr__(self):
             return "NotSelfDisplaying"
-        
+
         def _ipython_display_(self):
             raise NotImplementedError
-    
+
     save_enabled = f.ipython_display_formatter.enabled
     f.ipython_display_formatter.enabled = True
-    
+
     yes = SelfDisplaying()
     no = NotSelfDisplaying()
 
@@ -444,7 +444,7 @@ def _repr_png_(self):
             return 'should-be-overwritten'
         def _repr_html_(self):
             return '<b>hi!</b>'
-    
+
     f = get_ipython().display_formatter
     html_f = f.formatters['text/html']
     save_enabled = html_f.enabled
@@ -507,7 +507,7 @@ def _repr_mimebundle_(self, include=None, exclude=None):
                 }
             }
             return (data, metadata)
-    
+
     f = get_ipython().display_formatter
     obj = HasReprMimeMeta()
     d, md = f.format(obj)
@@ -529,3 +529,18 @@ def _repr_mimebundle_(self, include=None, exclude=None):
     obj = BadReprMime()
     d, md = f.format(obj)
     assert "text/plain" in d
+
+
+def test_custom_repr_namedtuple_partialmethod():
+    from functools import partialmethod
+    from typing import NamedTuple
+
+    class Foo(NamedTuple):
+        ...
+
+    Foo.__repr__ = partialmethod(lambda obj: "Hello World")
+    foo = Foo()
+
+    f = PlainTextFormatter()
+    assert f.pprint
+    assert f(foo) == "Hello World"
diff --git a/IPython/lib/pretty.py b/IPython/lib/pretty.py
index 631445b24e3..6cf3c48e116 100644
--- a/IPython/lib/pretty.py
+++ b/IPython/lib/pretty.py
@@ -406,8 +406,16 @@ def pretty(self, obj):
                             meth = cls._repr_pretty_
                             if callable(meth):
                                 return meth(obj, self, cycle)
-                        if cls is not object \
-                                and callable(cls.__dict__.get('__repr__')):
+                        if (
+                            cls is not object
+                            # check if cls defines __repr__
+                            and "__repr__" in cls.__dict__
+                            # check if __repr__ is callable.
+                            # Note: we need to test getattr(cls, '__repr__')
+                            #   instead of cls.__dict__['__repr__']
+                            #   in order to work with descriptors like partialmethod,
+                            and callable(getattr(cls, "__repr__", None))
+                        ):
                             return _repr_pprint(obj, self, cycle)
 
             return _default_pprint(obj, self, cycle)


Code review comments:
diff_hunk: @@ -406,8 +406,16 @@ def pretty(self, obj):
                             meth = cls._repr_pretty_
                             if callable(meth):
                                 return meth(obj, self, cycle)
-                        if cls is not object \
-                                and callable(cls.__dict__.get('__repr__')):
+                        if (
+                            cls is not object
+                            # check if cls defines __repr__
+                            and "__repr__" in cls.__dict__
+                            # check if __repr__ is callable.
+                            # Note: we need to test getattr(cls, '__repr__')
+                            #   instead of cls.__dict__['__repr__']
+                            #   in order to work with descriptors like partialmethod,
+                            and callable(getattr(cls, "__repr__", None))
path: IPython/lib/pretty.py
text: @user1:
I think we have a `_safe_getattr` that tries to detect mock objects that always return mock objects that always have existing methods.