In the PrettyPrinter._format method, since self._dispatch has dict.__repr__ [key] mapped to self._pprint_dict [value] the elif block is not needed. Its work is already being done by the if block above, which searches self._dispatch to fetch the appropriate value.

This change makes a difference in semantics (perhaps a good one) in this case:

import pprint

class MyDict(dict):
    def __repr__(self):
        return 'I do my own thing'*50

if __name__ == '__main__':
    d=MyDict()
    for i in range(50):
        d['a%s'%i] = i

    pprint.pprint(d)

Before the change MyDict.__repr__ is ignored and the dict contents are printed. After the change it prints "I do my own thing" 50 times.

Yes, this code was kept for backward compatibility when the dispatch mapping was added.

Adding serhiy.storchaka to nosy list since it looks like he introduced the isinstance check on purpose (see bpo-23741).

Though bpo-23741 asserts that no behavior was changed with the patch applied then, reading through the change leads me to think this did change for cases like the MyDict example from iritkatriel.

The intent of the original code was that MyDict.__repr__ would be used; only the small handful of "known" reprs would be handled specially.

bpo-23741 did not change the behavior. The specified code was added specially to preserve the existing behavior. For other base types the condition was like issubclass(typ, list) and r is list.__repr__, but for dict it was just issubclass(typ, dict).

Actually the initial behavior was changed by bad3c88 (no issue number), with adding support of OrderedDict.

Ah, good find! It's been so long since the first version of that code, but the implementation was surprisingly nuanced.

@palak Kumar Jha, you created a PR for this ticket but it seems that your GitHub account is inactive/has been removed. Do you have a new GitHub account? Are you still interested in continuing on this PR?

I went ahead and closed the PR. Either @palak Kumar Jha or someone else can create a new PR. The suggestions in the original PR should be addresses.

Thanks, I'll create a new PR and credit @palak Kumar Jha as co-author.

While writing the tests I see that it's even more interesting:

(Pdb) dd = MyDict()
(Pdb) pprint.pformat(dd)
'{}'
(Pdb) pprint.saferepr(dd)
'I do my own thing ....'

And that is why the original code was checking not only for the type, but the actual __repr__ method itself.

I think the current behavior is broken.

I realize it might break some corner cases, but I really think we should re-write pprint to use functools.singledispatch. Or if the breakage isn't acceptable, abandon it and create a new module that does use singledispatch. That way it would be easily extensible.

pprint currently works with a dispatch table (_dispatch), it's just hidden.

@fdrake: From the comments: "If you find it useful, thank small children who sleep at night". I assume they're not small anymore!

import pprint

class MyDict(dict):
    def __repr__(self):
        return '*'*len(dict.__repr__(self))

if __name__ == '__main__':
    d=MyDict({})
    print('pprint.pformat(d):\n%s' % pprint.pformat(d))
    print('pprint.pformat(d, width=1, indent=0):\n%s' % pprint.pformat(d, width=1, indent=0))

Output:

pprint.pformat(d):
**
pprint.pformat(d, width=1, indent=0):
{}

I've created a new PR, with two commits. The first has just tests added, which show the problems and assert that the bug is there. The second has the fix and update to the tests. Let me know what you think.

Only 3.8 - 3.10 would be eligible for this fix. 3.7 is getting only security fixes.

New changeset 582f137 by Irit Katriel in branch 'master':
bpo-39994: Fix pprint handling of dict subclasses that override __repr__ (GH-21892)
582f137

Thank you Serhiy. This ticket can be closed now.