bpo-28850: Fix PrettyPrinter.format overrides ignored for contents of small containers (pythonGH-22120)

iritkatriel · web-flow · commit ff420f0e08a2 · 2020-11-23T15:31:31.000+02:00
diff --git a/Lib/pprint.py b/Lib/pprint.py
@@ -64,15 +64,15 @@ def pp(object, *args, sort_dicts=False, **kwargs):
 
 def saferepr(object):
     """Version of repr() which can handle recursive data structures."""
-    return _safe_repr(object, {}, None, 0, True)[0]
+    return PrettyPrinter()._safe_repr(object, {}, None, 0)[0]
 
 def isreadable(object):
     """Determine if saferepr(object) is readable by eval()."""
-    return _safe_repr(object, {}, None, 0, True)[1]
+    return PrettyPrinter()._safe_repr(object, {}, None, 0)[1]
 
 def isrecursive(object):
     """Determine if object requires a recursive representation."""
-    return _safe_repr(object, {}, None, 0, True)[2]
+    return PrettyPrinter()._safe_repr(object, {}, None, 0)[2]
 
 class _safe_key:
     """Helper function for key functions when sorting unorderable objects.
@@ -435,7 +435,7 @@ def format(self, object, context, maxlevels, level):
         and flags indicating whether the representation is 'readable'
         and whether the object represents a recursive construct.
         """
-        return _safe_repr(object, context, maxlevels, level, self._sort_dicts)
+        return self._safe_repr(object, context, maxlevels, level)
 
     def _pprint_default_dict(self, object, stream, indent, allowance, context, level):
         if not len(object):
@@ -518,77 +518,79 @@ def _pprint_user_string(self, object, stream, indent, allowance, context, level)
 
     _dispatch[_collections.UserString.__repr__] = _pprint_user_string
 
-# Return triple (repr_string, isreadable, isrecursive).
+    def _safe_repr(self, object, context, maxlevels, level):
+        # Return triple (repr_string, isreadable, isrecursive).
+        typ = type(object)
+        if typ in _builtin_scalars:
+            return repr(object), True, False
 
-def _safe_repr(object, context, maxlevels, level, sort_dicts):
-    typ = type(object)
-    if typ in _builtin_scalars:
-        return repr(object), True, False
-
-    r = getattr(typ, "__repr__", None)
-    if issubclass(typ, dict) and r is dict.__repr__:
-        if not object:
-            return "{}", True, False
-        objid = id(object)
-        if maxlevels and level >= maxlevels:
-            return "{...}", False, objid in context
-        if objid in context:
-            return _recursion(object), False, True
-        context[objid] = 1
-        readable = True
-        recursive = False
-        components = []
-        append = components.append
-        level += 1
-        if sort_dicts:
-            items = sorted(object.items(), key=_safe_tuple)
-        else:
-            items = object.items()
-        for k, v in items:
-            krepr, kreadable, krecur = _safe_repr(k, context, maxlevels, level, sort_dicts)
-            vrepr, vreadable, vrecur = _safe_repr(v, context, maxlevels, level, sort_dicts)
-            append("%s: %s" % (krepr, vrepr))
-            readable = readable and kreadable and vreadable
-            if krecur or vrecur:
-                recursive = True
-        del context[objid]
-        return "{%s}" % ", ".join(components), readable, recursive
-
-    if (issubclass(typ, list) and r is list.__repr__) or \
-       (issubclass(typ, tuple) and r is tuple.__repr__):
-        if issubclass(typ, list):
+        r = getattr(typ, "__repr__", None)
+        if issubclass(typ, dict) and r is dict.__repr__:
             if not object:
-                return "[]", True, False
-            format = "[%s]"
-        elif len(object) == 1:
-            format = "(%s,)"
-        else:
-            if not object:
-                return "()", True, False
-            format = "(%s)"
-        objid = id(object)
-        if maxlevels and level >= maxlevels:
-            return format % "...", False, objid in context
-        if objid in context:
-            return _recursion(object), False, True
-        context[objid] = 1
-        readable = True
-        recursive = False
-        components = []
-        append = components.append
-        level += 1
-        for o in object:
-            orepr, oreadable, orecur = _safe_repr(o, context, maxlevels, level, sort_dicts)
-            append(orepr)
-            if not oreadable:
-                readable = False
-            if orecur:
-                recursive = True
-        del context[objid]
-        return format % ", ".join(components), readable, recursive
-
-    rep = repr(object)
-    return rep, (rep and not rep.startswith('<')), False
+                return "{}", True, False
+            objid = id(object)
+            if maxlevels and level >= maxlevels:
+                return "{...}", False, objid in context
+            if objid in context:
+                return _recursion(object), False, True
+            context[objid] = 1
+            readable = True
+            recursive = False
+            components = []
+            append = components.append
+            level += 1
+            if self._sort_dicts:
+                items = sorted(object.items(), key=_safe_tuple)
+            else:
+                items = object.items()
+            for k, v in items:
+                krepr, kreadable, krecur = self.format(
+                    k, context, maxlevels, level)
+                vrepr, vreadable, vrecur = self.format(
+                    v, context, maxlevels, level)
+                append("%s: %s" % (krepr, vrepr))
+                readable = readable and kreadable and vreadable
+                if krecur or vrecur:
+                    recursive = True
+            del context[objid]
+            return "{%s}" % ", ".join(components), readable, recursive
+
+        if (issubclass(typ, list) and r is list.__repr__) or \
+           (issubclass(typ, tuple) and r is tuple.__repr__):
+            if issubclass(typ, list):
+                if not object:
+                    return "[]", True, False
+                format = "[%s]"
+            elif len(object) == 1:
+                format = "(%s,)"
+            else:
+                if not object:
+                    return "()", True, False
+                format = "(%s)"
+            objid = id(object)
+            if maxlevels and level >= maxlevels:
+                return format % "...", False, objid in context
+            if objid in context:
+                return _recursion(object), False, True
+            context[objid] = 1
+            readable = True
+            recursive = False
+            components = []
+            append = components.append
+            level += 1
+            for o in object:
+                orepr, oreadable, orecur = self.format(
+                    o, context, maxlevels, level)
+                append(orepr)
+                if not oreadable:
+                    readable = False
+                if orecur:
+                    recursive = True
+            del context[objid]
+            return format % ", ".join(components), readable, recursive
+
+        rep = repr(object)
+        return rep, (rep and not rep.startswith('<')), False
 
 _builtin_scalars = frozenset({str, bytes, bytearray, int, float, complex,
                               bool, type(None)})
@@ -604,7 +606,7 @@ def _perfcheck(object=None):
         object = [("string", (1, 2), [3, 4], {5: 6, 7: 8})] * 100000
     p = PrettyPrinter()
     t1 = time.perf_counter()
-    _safe_repr(object, {}, None, 0, True)
+    p._safe_repr(object, {}, None, 0, True)
     t2 = time.perf_counter()
     p.pformat(object)
     t3 = time.perf_counter()
diff --git a/Lib/test/test_pprint.py b/Lib/test/test_pprint.py
@@ -453,12 +453,23 @@ class AdvancedNamespace(types.SimpleNamespace): pass
                   dog=8)""")
 
     def test_subclassing(self):
+        # length(repr(obj)) > width
         o = {'names with spaces': 'should be presented using repr()',
              'others.should.not.be': 'like.this'}
         exp = """\
 {'names with spaces': 'should be presented using repr()',
  others.should.not.be: like.this}"""
-        self.assertEqual(DottedPrettyPrinter().pformat(o), exp)
+
+        dotted_printer = DottedPrettyPrinter()
+        self.assertEqual(dotted_printer.pformat(o), exp)
+
+        # length(repr(obj)) < width
+        o1 = ['with space']
+        exp1 = "['with space']"
+        self.assertEqual(dotted_printer.pformat(o1), exp1)
+        o2 = ['without.space']
+        exp2 = "[without.space]"
+        self.assertEqual(dotted_printer.pformat(o2), exp2)
 
     def test_set_reprs(self):
         self.assertEqual(pprint.pformat(set()), 'set()')
diff --git a/Misc/NEWS.d/next/Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst b/Misc/NEWS.d/next/Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst
@@ -0,0 +1 @@
+Fix :meth:`pprint.PrettyPrinter.format` overrides being ignored for contents of small containers. The :func:`pprint._safe_repr` function was removed.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fix :meth:`pprint.PrettyPrinter.format` overrides being ignored for contents of small containers. The :func:`pprint._safe_repr` function was removed.