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

Regression in Python 3: Subclassing PrettyPrinter.format doesn't work anymore #73036

Closed
mice mannequin opened this issue Dec 1, 2016 · 4 comments
Closed

Regression in Python 3: Subclassing PrettyPrinter.format doesn't work anymore #73036

mice mannequin opened this issue Dec 1, 2016 · 4 comments
Labels
3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@mice
Copy link
Mannequin

mice mannequin commented Dec 1, 2016

BPO 28850
Nosy @doerwalter, @rhettinger, @taleinat, @serhiy-storchaka, @zhangyangyu, @iritkatriel
PRs
  • bpo-28850: Fix PrettyPrinter.format overrides being ignored for contents of small containers #22120
  • 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 = None
    closed_at = <Date 2020-11-23.13:34:04.787>
    created_at = <Date 2016-12-01.11:45:23.870>
    labels = ['type-bug', 'library', '3.10']
    title = "Regression in Python 3: Subclassing PrettyPrinter.format doesn't work anymore"
    updated_at = <Date 2020-11-23.13:34:04.786>
    user = 'https://bugs.python.org/mice'

    bugs.python.org fields:

    activity = <Date 2020-11-23.13:34:04.786>
    actor = 'taleinat'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-11-23.13:34:04.787>
    closer = 'taleinat'
    components = ['Library (Lib)']
    creation = <Date 2016-12-01.11:45:23.870>
    creator = 'mic_e'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 28850
    keywords = ['patch']
    message_count = 4.0
    messages = ['282159', '376465', '381667', '381668']
    nosy_count = 9.0
    nosy_names = ['doerwalter', 'anthonybaxter', 'rhettinger', 'taleinat', 'markhirota', 'serhiy.storchaka', 'mic_e', 'xiang.zhang', 'iritkatriel']
    pr_nums = ['22120']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28850'
    versions = ['Python 3.10']

    @mice
    Copy link
    Mannequin Author

    mice mannequin commented Dec 1, 2016

    This issue was previously addressed and fixed here:

    http://bugs.python.org/issue1351692

    When subclassing PrettyPrinter, overriding the format() method should allow users to define custom pretty-printers.

    However, for objects whose repr is short, format() is not called for the individual members.

    Example code that reproduces the issue is as follows:

    import pprint
    import sys
    
    pprint.pprint(sys.version_info)
    
    class MyPrettyPrinter(pprint.PrettyPrinter):
        def format(self, object, context, maxlevels, level):
            if isinstance(object, int):
                return hex(object), True, False
            else:
                return pprint.PrettyPrinter.format(self, object, context, maxlevels, level)

    MyPrettyPrinter().pprint(10)
    MyPrettyPrinter().pprint([10])

    When run with different versions of Python:

    sys.version_info(major=2, minor=7, micro=11, releaselevel='final', serial=0)
    0xa
    [0xa]

    (3, 0, 1, 'final', 0)
    0xa
    [10]

    sys.version_info(major=3, minor=2, micro=5, releaselevel='final', serial=0)
    0xa
    [10]
    
    sys.version_info(major=3, minor=4, micro=4, releaselevel='final', serial=0)
    0xa
    [10]
    
    sys.version_info(major=3, minor=6, micro=0, releaselevel='beta', serial=4)
    0xa
    [10]

    You'll notice that the regression exists in all versions >= 3.0,
    even though the commit that fixed the issue is still in the log (2008-01-20):
    https://hg.python.org/cpython/log/3.0/Lib/pprint.py

    I have not had the time to look at the source of the issue or provide a fix; I might do so tonight.

    @mice mice mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Dec 1, 2016
    @zhangyangyu zhangyangyu added the 3.7 (EOL) end of life label Dec 2, 2016
    @iritkatriel
    Copy link
    Member

    This issue only impacts container objects where the len(repr(o)) is less than width. If the length is greater than width, containers are handled by a different code path which is covered by a unit test and works correctly.

    For this case, indeed it works in Python 2 but not in Python 3. The PrettyPrinter._format code has been refactored quite a lot with respect to handling of containers.

    In both Python 2 and 3 this happens:

    PrettyPrinter.pprint([10]) calls
    PrettyPrinter._format([10]) which calls
    PrettyPrinter._repr([10]) which calls
    MyPrettyPrinter.format([10]) which calls
    PrettyPrinter.format([10]) which calls
    _safe_repr([10]) which calls
    _safe_repr(10)
    returns 10
    returns [10]
    returns [10]
    returns [10]
    returns [10]

    But then they diverge - in Python 3 the [10] is returned as the result, but in Python 2 there is another piece of code (starting here

    if ((issubclass(typ, list) and r is list.__repr__) or
    ) which overrides this result and recalculates the representation for containers. In our case it does:

    	since issubclass(type([10]), list):
    		\# ignore the [10] calculated above, now call
    		self.\_format(10) which calls
    			PrettyPrinter.\_repr(10) which calls
    				MyPrettyPrinter.format(10)
    				returns 0xa
    			returns 0xa
    		returns 0xa
    	returns [0xa]
    

    This explains the difference between Python 2 and 3.

    As to why the first calculation returns [10] and not [0xa]:
    This is because _safe_repr is defined at module scope, so once it is called we are no longer on the PrettyPrinter instance, and the format() override is no longer accessible. When _safe_repr needs to recurse on container contents, it calls itself.

    I think the solution is to make _safe_repr a method of PrettyPrinter, and make it call self.format() when it needs to recurse. The default self.format() just calls _safe_repr, so when there is no override the result is the same.

    The PR's diff looks substantial, but that's mostly due to the indentation of _safe_repr code. To make it easier to review, I split it into several commits, where the indentation is done in one commit as a noop change. The other commits have much smaller diffs.

    @iritkatriel iritkatriel added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes and removed 3.7 (EOL) end of life labels Sep 6, 2020
    @taleinat
    Copy link
    Contributor

    New changeset ff420f0 by Irit Katriel in branch 'master':
    bpo-28850: Fix PrettyPrinter.format overrides ignored for contents of small containers (GH-22120)
    ff420f0

    @taleinat
    Copy link
    Contributor

    Thanks for the report, Michael!

    Thanks for the fix, Irit!

    @taleinat taleinat removed 3.8 only security fixes 3.9 only security fixes labels Nov 23, 2020
    @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
    3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants