[Patches] [ python-Patches-1713041 ] fix for 1712742: corrects pprint's handling of 'depth'
SourceForge.net
noreply at sourceforge.net
Fri Jun 29 16:12:00 CEST 2007
Patches item #1713041, was opened at 2007-05-04 15:13
Message generated for change (Comment added) made by draghuram
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1713041&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.6
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Raghuram Devarakonda (draghuram)
Assigned to: Neal Norwitz (nnorwitz)
Summary: fix for 1712742: corrects pprint's handling of 'depth'
Initial Comment:
The patch is really simple. Just change the way "maxlevels" is checked. I couldn't find a way to have a test case for this bug.
----------------------------------------------------------------------
>Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-06-29 10:12
Message:
Logged In: YES
user_id=984087
Originator: YES
Neal, can you please apply the patch? - Thanks.
----------------------------------------------------------------------
Comment By: Rodrigo Bernardo Pimentel (errebepe)
Date: 2007-06-01 10:03
Message:
Logged In: YES
user_id=374783
Originator: NO
Neal: sorry, I understood pprint.patch was the "current", valid patch, and
the object of the most recent discussion. This is the one I was referring
to.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2007-06-01 02:48
Message:
Logged In: YES
user_id=33168
Originator: NO
Rodrigo, which patch are you giving +1?
Sorry Raghu, I'm getting behind. :-(
I'll try to take a look at this. If everything is consistent (docs, code,
and tests) and has sane semantics, there shouldn't be anything else
necessary. I just need to get the time to apply. Or someone else with
commit privileges could apply this as well. If I don't get to this within
a week or so, feel free to ping me via mail.
----------------------------------------------------------------------
Comment By: Rodrigo Bernardo Pimentel (errebepe)
Date: 2007-05-30 23:06
Message:
Logged In: YES
user_id=374783
Originator: NO
+1 on the patch (test, code and doc). In particular, if depth == 0 is not
allowed, I think the patched behaviour is the expected one, so this is
actually a bug fix rather than a change in the module's semantic.
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-18 17:16
Message:
Logged In: YES
user_id=984087
Originator: YES
Neal, is there anything else you want me to do for the patch?
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-13 11:09
Message:
Logged In: YES
user_id=984087
Originator: YES
josm, please feel free to go to python-dev if you think community input is
required. I personally don't think that it is warranted.
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-05-13 08:14
Message:
Logged In: YES
user_id=1776568
Originator: NO
What is intuitive is a matter of taste.
Some people like to count from zero, like many programmers do.
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-13 07:56
Message:
Logged In: YES
user_id=984087
Originator: YES
josm, merely changing (fixing) the handling of "depth" parameter is not a
spec change. The patch makes pprint behave in a way that is intuitive from
the meaning of "depth".
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-05-12 10:09
Message:
Logged In: YES
user_id=1776568
Originator: NO
Yes, the doc seems to be wrong, and I agree with you
changing wrong doc is always right thing to do,
but this patch changed the way how pprint handles 'depth'.
I think I can call this as 'a spec change'.
changing some existent module's behavior is
something that needs to be done carefully.
Don't we need a consensus among community?
No one cares so much how pprint works?
could be ...
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-10 17:41
Message:
Logged In: YES
user_id=984087
Originator: YES
josm> I just thought if changing the doc itself would cause
josm> some problem, we have to fix the code to work the way
josm> the doc says.
Not if the doc is wrong.
josm> Don't we have to get someone's approval to change the
josm> spec?
We are not changing the spec here.
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-05-10 17:37
Message:
Logged In: YES
user_id=1776568
Originator: NO
That test case was for pp2.diff.
I just thought if changing the doc itself would cause some problem,
we have to fix the code to work the way the doc says.
Don't we have to get someone's approval to change the spec?
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-10 10:13
Message:
Logged In: YES
user_id=984087
Originator: YES
File Added: pprint.patch
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-10 10:13
Message:
Logged In: YES
user_id=984087
Originator: YES
josm, I don't think there is need for a new test case. The one in the
patch correctly tests the code change. As Neal pointed out, the doc is
wrong to begin with.
I updated the patch with the doc change. Note that I replaced the current
example with a very simple one. I believe the current one is overly
complicated for this purpose. Comments are welcome, of course.
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-05-10 07:56
Message:
Logged In: YES
user_id=1776568
Originator: NO
I agree. The code and the doc should be consistent.
New test would be
Index: Lib/test/test_pprint.py
===================================================================
--- Lib/test/test_pprint.py (revision 55223)
+++ Lib/test/test_pprint.py (working copy)
@@ -195,7 +195,27 @@
others.should.not.be: like.this}"""
self.assertEqual(DottedPrettyPrinter().pformat(o), exp)
+ def test_depth(self):
+ nested_tuple = (1, (2, (3, (4, (5, 6)))))
+ nested_dict = {1: {2: {3: {4: {5: {6: 6}}}}}}
+ nested_list = [1, [2, [3, [4, [5, [6, []]]]]]]
+ self.assertEqual(pprint.pformat(nested_tuple),
repr(nested_tuple))
+ self.assertEqual(pprint.pformat(nested_dict), repr(nested_dict))
+ self.assertEqual(pprint.pformat(nested_list), repr(nested_list))
+
+ result_tuple = {'lv1': '(...)',
+ 'lv2': '(1, (...))'}
+ result_dict = {'lv1': '{...}',
+ 'lv2': '{1: {...}}'}
+ result_list = {'lv1': '[...]',
+ 'lv2': '[1, [...]]'}
+ for i in [1, 2]:
+ key = 'lv' + `i`
+ self.assertEqual(pprint.pformat(nested_tuple, depth=i),
result_tuple[key])
+ self.assertEqual(pprint.pformat(nested_dict, depth=i),
result_dict[key])
+ self.assertEqual(pprint.pformat(nested_list, depth=i),
result_list[key])
+
class DottedPrettyPrinter(pprint.PrettyPrinter):
def format(self, object, context, maxlevels, level):
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2007-05-10 02:18
Message:
Logged In: YES
user_id=33168
Originator: NO
Yes, that's the example I'm referring to. In the doc, it shows 5 numbers
and one ... for 6. Without your patch it shows 7 numbers and with your
patch it shows 6 numbers. So even with your patch the doc and code don't
agree (they are off by one, rather than 2 as is currently the case).
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-09 10:44
Message:
Logged In: YES
user_id=984087
Originator: YES
Hi Neal,
I assume you are referring to the example
>>> import parser
>>> tup = parser.ast2tuple(
... parser.suite(open('pprint.py').read()))[1][1][1]
>>> pp = pprint.PrettyPrinter(depth=6)
>>> pp.pprint(tup)
in the document. Is that correct? I ran this example with and without my
patch. Without the update, the example printed 7 levels which is one level
too deep. With the patch, it printed 6 levels, which seems correct to me.
# without patch. prints 7 levels.
$ ../python/python testdoc.py
(268, (269, (326, (303, (304, (305, (306, (...))))))))
# with patch. prints 6 levels.
$ ../python/python testdoc.py
(268, (269, (326, (303, (304, (305, (...)))))))
I am attaching the file testdoc.py which contains the doc example. I just
wanted to confirm that this is what you are referring to.
File Added: testdoc.py
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2007-05-09 02:56
Message:
Logged In: YES
user_id=33168
Originator: NO
I notice in the doc an example which doesn't work with this patch. It
still prints one level too deep. The doc seems correct to me, but I don't
have strong feelings any way. The attached patch makes the doc example
work as expected. The doc should really be updated with an example more
like:
>>> pp = pprint.PrettyPrinter(depth=6)
>>> pp.pprint((1, (2, (3, (4, (5, (6, 7)))))))
(1, (2, (3, (4, (5, (...))))))
>>> pp = pprint.PrettyPrinter(depth=1)
>>> pp.pprint(1)
1
>>> pp.pprint([1])
[...]
The updated patch causes the new tests to fail. Could you update the
test/code/doc to all be consistent?
File Added: pp2.diff
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-07 10:49
Message:
Logged In: YES
user_id=984087
Originator: YES
josm, thanks for the test case. I incorporated it into the patch. BTW, I
changed "str" to "repr" as I think "repr" is closer to what pformat does.
File Added: pprint.patch
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-05-05 05:02
Message:
Logged In: YES
user_id=1776568
Originator: NO
Your patch looks good and worked fine.
Wouldn't it be nice to add a test case for this problem to
test_pprint.py?
The following is just draft patch to add the test case.
Index: Lib/test/test_pprint.py
===================================================================
--- Lib/test/test_pprint.py (revision 55144)
+++ Lib/test/test_pprint.py (working copy)
@@ -195,7 +195,22 @@
others.should.not.be: like.this}"""
self.assertEqual(DottedPrettyPrinter().pformat(o), exp)
+ def test_depth(self):
+ nested_tuple = (1, (2, (3, (4, (5, 6)))))
+ nested_dict = {1: {2: {3: {4: {5: {6: 6}}}}}}
+ nested_list = [1, [2, [3, [4, [5, [6, []]]]]]]
+ self.assertEqual(pprint.pformat(nested_tuple),
str(nested_tuple))
+ self.assertEqual(pprint.pformat(nested_dict), str(nested_dict))
+ self.assertEqual(pprint.pformat(nested_list), str(nested_list))
+
+ lv1_tuple = '(1, (...))'
+ lv1_dict = '{1: {...}}'
+ lv1_list = '[1, [...]]'
+ self.assertEqual(pprint.pformat(nested_tuple, depth=1),
lv1_tuple)
+ self.assertEqual(pprint.pformat(nested_dict, depth=1), lv1_dict)
+ self.assertEqual(pprint.pformat(nested_list, depth=1), lv1_list)
+
----------------------------------------------------------------------
Comment By: Raghuram Devarakonda (draghuram)
Date: 2007-05-04 15:15
Message:
Logged In: YES
user_id=984087
Originator: YES
File Added: pprint_test.py
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1713041&group_id=5470
More information about the Patches
mailing list