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

pprint._safe_repr is not general enough in one instance #48226

Closed
erickt mannequin opened this issue Sep 26, 2008 · 12 comments
Closed

pprint._safe_repr is not general enough in one instance #48226

erickt mannequin opened this issue Sep 26, 2008 · 12 comments
Assignees
Labels
stdlib Python modules in the Lib dir

Comments

@erickt
Copy link
Mannequin

erickt mannequin commented Sep 26, 2008

BPO 3976
Nosy @birkenfeld, @rhettinger, @abalkin, @pitrou, @mitsuhiko

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/rhettinger'
closed_at = <Date 2009-11-19.01:07:22.972>
created_at = <Date 2008-09-26.16:14:59.303>
labels = ['library']
title = 'pprint._safe_repr is not general enough in one instance'
updated_at = <Date 2010-10-08.19:00:39.241>
user = 'https://bugs.python.org/erickt'

bugs.python.org fields:

activity = <Date 2010-10-08.19:00:39.241>
actor = 'rbp'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2009-11-19.01:07:22.972>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2008-09-26.16:14:59.303>
creator = 'erickt'
dependencies = []
files = []
hgrepos = []
issue_num = 3976
keywords = []
message_count = 12.0
messages = ['73858', '73860', '73864', '78682', '79278', '92904', '92905', '92906', '92907', '92908', '95460', '118217']
nosy_count = 10.0
nosy_names = ['georg.brandl', 'rhettinger', 'belopolsky', 'pitrou', 'idadesub', 'erickt', 'LambertDW', 'rbp', 'aronacher', 'robert.kern']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue3976'
versions = ['Python 3.1', 'Python 3.2']

@erickt
Copy link
Mannequin Author

erickt mannequin commented Sep 26, 2008

I've run into a case where pprint isn't able to print out a particular
data structure, and have distilled it down to a simple example:

import pprint

class A:
    pass

pprint.pprint({A(): 1, A(): 2})

Which throws this exception:

Traceback (most recent call last):
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 272, in _safe_repr
    items = sorted(items)
TypeError: unorderable types: A() < A()

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "foo.py", line 6, in <module>
    pprint.pprint({A(): 1, A(): 2})
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 55, in pprint
    printer.pprint(object)
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 106, in pprint
    self._format(object, self._stream, 0, 0, {}, 0)
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 129, in _format
    rep = self._repr(object, context, level - 1)
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 216, in _repr
    self._depth, level)
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 228, in format
    return _safe_repr(object, context, maxlevels, level)
  File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.0/lib/python3
.0/pprint.py", line 277, in _safe_repr
    items = sorted(items, key=sortkey)
TypeError: unorderable types: A() < A()

This is happening because of this block of code:

    try:
        items = sorted(items)
    except TypeError:
        def sortkey(item):
            key, value = item
            return str(type(key)), key, value
        items = sorted(items, key=sortkey)

The exception block is trying to sort the items again, but in this
instance, it's still not orderable. Could we get _safe_repr to at least
give up on sorting at this point? Or, we could try just falling back to
sorting according to the class name, with:

    try:
        items = sorted(items)
    except TypeError:
        def sortkey(item):
            key, value = item
            return str(type(key)), key, value
        try:
            items = sorted(items, key=sortkey)
        except TypeError:
            def sortkey(item):
                key, value = item
                return str(type(key))

That would at least give some ordering to the output. Unfortunately, in
this case it's a shame that we don't have the cmp function any more,
because then we could just fall back to giving up on the ordering for
just certain unorderable keys, but still have sorted output for
orderable keys. I thought maybe we could test if the key and value have
__lt__, but it looks like all classes now have that function, even if
the user didn't implement it. In the long run though, I suppose the case
where you have mixed types in a dict there's no sensible ordering
anyway.

@erickt erickt mannequin added the stdlib Python modules in the Lib dir label Sep 26, 2008
@pitrou
Copy link
Member

pitrou commented Sep 26, 2008

Another solution would be to separate the dict items by key type, try to
sort each items list with separate fallback on onsorted. Then merge the
whole thing, ordered by key type name.

@idadesub
Copy link
Mannequin

idadesub mannequin commented Sep 26, 2008

fyi, I found another case where pprint needs a "safe sort", this is
when you have a list that contains a dictionary. Anyway, Here's a
simple patch that creates a _safe_sorted function that implements the
fallback:

--- /opt/local/lib/python3.0/pprint.py	2008-09-26 09:35:21.000000000 -0700
+++ /tmp/pprint.py	2008-09-26 09:35:13.000000000 -0700
@@ -145,7 +145,7 @@
                 if length:
                     context[objid] = 1
                     indent = indent + self._indent_per_level
-                    items  = sorted(object.items())
+                    items  = _safe_sorted(object.items())
                     key, ent = items[0]
                     rep = self._repr(key, context, level)
                     write(rep)
@@ -267,14 +267,7 @@
         append = components.append
         level += 1
         saferepr = _safe_repr
-        items = object.items()
-        try:
-            items = sorted(items)
-        except TypeError:
-            def sortkey(item):
-                key, value = item
-                return str(type(key)), key, value
-            items = sorted(items, key=sortkey)
+        items = _safe_sorted(object.items())
         for k, v in items:
             krepr, kreadable, krecur = saferepr(k, context, maxlevels, level)
             vrepr, vreadable, vrecur = saferepr(v, context, maxlevels, level)
@@ -321,6 +314,20 @@
     rep = repr(object)
     return rep, (rep and not rep.startswith('<')), False

+def _safe_sorted(items):
+ try:
+ return sorted(items)
+ except TypeError:
+ def sortkey(item):
+ key, value = item
+ return str(type(key)), key, value
+ try:
+ return sorted(items, key=sortkey)
+ except TypeError:
+ def sortkey(item):
+ key, value = item
+ return str(type(key))
+ return sorted(items, key=sortkey)

 def _recursion(object):
     return ("<Recursion on %s with id=%s>"

One other thing to note is that I'm also aware that the yaml project
also has this problem, and they've got their own "safe_sorted"
function. It might be worthwhile formalizing this in a public function
in the api somewhere.

@pitrou
Copy link
Member

pitrou commented Jan 1, 2009

For this to be integrated, it should also add an unit test.

@abalkin
Copy link
Member

abalkin commented Jan 6, 2009

The proposed patch appears to give up sorting by key,value altogether if
there are a few incomparable items. It would be better to group items by
type and sort within each group. For example,
pprint({1:1,2:2,A():3,A():4}) should print int:int items in order.

@birkenfeld
Copy link
Member

Also note that this patch will not sort sequences of mixed types where
some types are intercomparable "correctly" (in the way that Python 2
did), e.g. for

{1:2, 2:3, 'a':4, 1.5: 5}

the 1.5 key will not be placed between the 1 and 2 keys.

I'm not aware of a way to implement that behavior without support for a
general comparison function instead of a DSU key function.

@rhettinger
Copy link
Contributor

I'm thinking that pprint should not try to sort unsortable items.

try:
items = sorted(items)
except TypeError:
items = list(items)

@rhettinger rhettinger self-assigned this Sep 20, 2009
@birkenfeld
Copy link
Member

OK, there *is* a way. Consider this:

class safe_key(object):
    __slots__ = ('obj',)

    def __init__(self, obj):
        self.obj = obj

    def __eq__(self, other):
        return self.obj.__eq__(other.obj)

    def __lt__(self, other):
        try:
            return self.obj < other.obj
        except TypeError:
            return id(type(self.obj)) < id(type(other.obj))


ls = [2, 1, 1.0, 1.5, 'a', 'c', 'b']
print(sorted(ls, key=safe_key))

@mitsuhiko
Copy link
Member

@georg: Instead of catching a TypeError i would rather call __gt__ /
__lt__ directly and check for NotImplemented. Python 2.x did not catch
TypeErrors either.

@mitsuhiko
Copy link
Member

Eg, something like this:

class safe_key(object):

    __slots__ = ('obj',)

    def __init__(self, obj):
        self.obj = obj

    def __eq__(self, other):
        return self.obj.__eq__(other.obj)

    def __lt__(self, other):
        rv = self.obj.__lt__(other.obj)
        if rv is NotImplemented:
            rv = id(type(self.obj)) < id(type(other.obj))
        return rv


ls = [2, 1, 1.0, 1.5, 'a', 'c', 'b']
print(sorted(ls, key=safe_key))

@rhettinger
Copy link
Contributor

Fixed. See r76389 and r76390.

@rbp
Copy link
Mannequin

rbp mannequin commented Oct 8, 2010

Armin: this has the problem that, if the object you're trying to compare is a class, self.obj.__lt__ expects a different number of parameters (i.e., it expects the instance). See bpo-10017 . Testing with "<" works.

@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
stdlib Python modules in the Lib dir
Projects
None yet
Development

No branches or pull requests

5 participants