From 0b849c23c8c3a26fe435bab1354e66afa7a921df Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sat, 5 Sep 2020 20:49:02 +0100 Subject: [PATCH 1/9] bpo-28850: add small objects to the subclassing unit test, expecting the buggy behaviour --- Lib/test/test_pprint.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lib/test/test_pprint.py b/Lib/test/test_pprint.py index 8ee18e8fef84f7..91b52c48cd6e9c 100644 --- a/Lib/test/test_pprint.py +++ b/Lib/test/test_pprint.py @@ -453,6 +453,7 @@ 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 = """\ @@ -460,6 +461,14 @@ def test_subclassing(self): others.should.not.be: like.this}""" self.assertEqual(DottedPrettyPrinter().pformat(o), exp) + # length(repr(obj)) < width + o1 = ['with space'] + exp1 = "['with space']" + self.assertEqual(DottedPrettyPrinter().pformat(o1), exp1) + o2 = ['without.space'] + exp2 = "['without.space']" # this is bug bpo-28850 + self.assertEqual(DottedPrettyPrinter().pformat(o2), exp2) + def test_set_reprs(self): self.assertEqual(pprint.pformat(set()), 'set()') self.assertEqual(pprint.pformat(set(range(3))), '{0, 1, 2}') From f6c2c199ec733d2fed17b46c9174568965a0f89a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sat, 5 Sep 2020 20:51:00 +0100 Subject: [PATCH 2/9] bpo-28850: indent _safe_repr under an 'if True:' as prep for making it part of the PrettyPrinter class (no functional change) --- Lib/pprint.py | 137 +++++++++++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 68 deletions(-) diff --git a/Lib/pprint.py b/Lib/pprint.py index 213998e3491ef7..8ee8af099d5cea 100644 --- a/Lib/pprint.py +++ b/Lib/pprint.py @@ -518,77 +518,78 @@ def _pprint_user_string(self, object, stream, indent, allowance, context, level) _dispatch[_collections.UserString.__repr__] = _pprint_user_string -# Return triple (repr_string, isreadable, isrecursive). +if True: + # Return triple (repr_string, isreadable, isrecursive). -def _safe_repr(object, context, maxlevels, level, sort_dicts): - 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 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): + 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 _builtin_scalars = frozenset({str, bytes, bytearray, int, float, complex, bool, type(None)}) From fcf7747ff9962c5aebae4d8ea7ebe35891aa95c1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sun, 6 Sep 2020 16:09:41 +0100 Subject: [PATCH 3/9] bpo-28850: make _safe_repr a method of PrettyPrinter --- Lib/pprint.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/Lib/pprint.py b/Lib/pprint.py index 8ee8af099d5cea..ed59595976a7b3 100644 --- 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,10 +518,8 @@ def _pprint_user_string(self, object, stream, indent, allowance, context, level) _dispatch[_collections.UserString.__repr__] = _pprint_user_string -if True: - # Return triple (repr_string, isreadable, isrecursive). - - def _safe_repr(object, context, maxlevels, level, sort_dicts): + 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 @@ -541,13 +539,13 @@ def _safe_repr(object, context, maxlevels, level, sort_dicts): components = [] append = components.append level += 1 - if sort_dicts: + if self._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) + krepr, kreadable, krecur = self._safe_repr(k, context, maxlevels, level) + vrepr, vreadable, vrecur = self._safe_repr(v, context, maxlevels, level) append("%s: %s" % (krepr, vrepr)) readable = readable and kreadable and vreadable if krecur or vrecur: @@ -579,7 +577,7 @@ def _safe_repr(object, context, maxlevels, level, sort_dicts): append = components.append level += 1 for o in object: - orepr, oreadable, orecur = _safe_repr(o, context, maxlevels, level, sort_dicts) + orepr, oreadable, orecur = self._safe_repr(o, context, maxlevels, level) append(orepr) if not oreadable: readable = False @@ -605,7 +603,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() From ad9686f2816ae28ec024c4deadee7bdf27a4f81e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sun, 6 Sep 2020 17:13:34 +0100 Subject: [PATCH 4/9] bpo-28850: make _safe_repr call self.format when it needs to recurse on container contents --- Lib/pprint.py | 6 +++--- Lib/test/test_pprint.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/pprint.py b/Lib/pprint.py index ed59595976a7b3..aed0642ce45bbe 100644 --- a/Lib/pprint.py +++ b/Lib/pprint.py @@ -544,8 +544,8 @@ def _safe_repr(self, object, context, maxlevels, level): else: items = object.items() for k, v in items: - krepr, kreadable, krecur = self._safe_repr(k, context, maxlevels, level) - vrepr, vreadable, vrecur = self._safe_repr(v, context, maxlevels, level) + 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: @@ -577,7 +577,7 @@ def _safe_repr(self, object, context, maxlevels, level): append = components.append level += 1 for o in object: - orepr, oreadable, orecur = self._safe_repr(o, context, maxlevels, level) + orepr, oreadable, orecur = self.format(o, context, maxlevels, level) append(orepr) if not oreadable: readable = False diff --git a/Lib/test/test_pprint.py b/Lib/test/test_pprint.py index 91b52c48cd6e9c..f368365eb4db86 100644 --- a/Lib/test/test_pprint.py +++ b/Lib/test/test_pprint.py @@ -466,7 +466,7 @@ def test_subclassing(self): exp1 = "['with space']" self.assertEqual(DottedPrettyPrinter().pformat(o1), exp1) o2 = ['without.space'] - exp2 = "['without.space']" # this is bug bpo-28850 + exp2 = "[without.space]" self.assertEqual(DottedPrettyPrinter().pformat(o2), exp2) def test_set_reprs(self): From 78262b176495e9bb92560118ae135e47cc5b2dbe Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2020 16:37:55 +0000 Subject: [PATCH 5/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst diff --git a/Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst b/Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst new file mode 100644 index 00000000000000..e4bcc863242f20 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst @@ -0,0 +1 @@ +Fix PrettyPrinter.format overrides being ignored for contents of small containers. \ No newline at end of file From 5de64c4e1e55d928abf48a8de45881aa0bff93f1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sun, 6 Sep 2020 18:01:46 +0100 Subject: [PATCH 6/9] Delete 2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst --- .../next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst diff --git a/Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst b/Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst deleted file mode 100644 index e4bcc863242f20..00000000000000 --- a/Misc/NEWS.d/next/Library/2020-09-06-16-37-54.bpo-issue28850.HJNggD.rst +++ /dev/null @@ -1 +0,0 @@ -Fix PrettyPrinter.format overrides being ignored for contents of small containers. \ No newline at end of file From 3d7ced7e83365416a600a32a5562a01d7df0a637 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2020 21:55:45 +0000 Subject: [PATCH 7/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst 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 new file mode 100644 index 00000000000000..e4bcc863242f20 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst @@ -0,0 +1 @@ +Fix PrettyPrinter.format overrides being ignored for contents of small containers. \ No newline at end of file From 2e1c705e79d631fc17e491427cf58563bf712205 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 5 Nov 2020 00:10:02 +0000 Subject: [PATCH 8/9] Zackery's review comments (the easy ones) --- Lib/pprint.py | 9 ++++++--- Lib/test/test_pprint.py | 8 +++++--- .../Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Lib/pprint.py b/Lib/pprint.py index aed0642ce45bbe..a8af50e5a68611 100644 --- a/Lib/pprint.py +++ b/Lib/pprint.py @@ -544,8 +544,10 @@ def _safe_repr(self, object, context, maxlevels, level): 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) + 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: @@ -577,7 +579,8 @@ def _safe_repr(self, object, context, maxlevels, level): append = components.append level += 1 for o in object: - orepr, oreadable, orecur = self.format(o, context, maxlevels, level) + orepr, oreadable, orecur = self.format( + o, context, maxlevels, level) append(orepr) if not oreadable: readable = False diff --git a/Lib/test/test_pprint.py b/Lib/test/test_pprint.py index f368365eb4db86..c4a8578a9fc8fb 100644 --- a/Lib/test/test_pprint.py +++ b/Lib/test/test_pprint.py @@ -459,15 +459,17 @@ def test_subclassing(self): 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(DottedPrettyPrinter().pformat(o1), exp1) + self.assertEqual(dotted_printer.pformat(o1), exp1) o2 = ['without.space'] exp2 = "[without.space]" - self.assertEqual(DottedPrettyPrinter().pformat(o2), exp2) + 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 index e4bcc863242f20..1000b12c84eaf7 100644 --- 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 @@ -1 +1 @@ -Fix PrettyPrinter.format overrides being ignored for contents of small containers. \ No newline at end of file +Fix :meth:`pprint.PrettyPrinter.format` overrides being ignored for contents of small containers. \ No newline at end of file From f46af4d8df92b2771710aea19cce059d7a7d7c17 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 23 Nov 2020 11:01:32 +0000 Subject: [PATCH 9/9] Update 2020-09-06-21-55-44.bpo-28850.HJNggD.rst --- .../next/Library/2020-09-06-21-55-44.bpo-28850.HJNggD.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 1000b12c84eaf7..fc6bd1d57e2ae7 100644 --- 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 @@ -1 +1 @@ -Fix :meth:`pprint.PrettyPrinter.format` overrides being ignored for contents of small containers. \ No newline at end of file +Fix :meth:`pprint.PrettyPrinter.format` overrides being ignored for contents of small containers. The :func:`pprint._safe_repr` function was removed.