Skip to content

gh-92546: Fix invalid call in pprint executed as a script#92560

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.

Sign up for GitHub

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

Closed
arhadthedev wants to merge 3 commits into from
Closed

gh-92546: Fix invalid call in pprint executed as a script #92560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa1e6b9
Fix invalid call in pprint runned as a script
arhadthedev May 9, 2022
d54161d
Fix the NEXT entry
arhadthedev May 9, 2022
b766d7a
Address the review
arhadthedev May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Lib/pprint.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -643,7 +643,7 @@ def _perfcheck(object=None):
object = [("string", (1, 2), [3, 4],{5: 6, 7: 8})] * 100000
p = PrettyPrinter()
t1 = time.perf_counter()
p._safe_repr(object,{}, None, 0, True)
p._safe_repr(object,{}, None, 0)
Copy link
Member

@iritkatrieliritkatrielMay 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for reporting. We typically commit a bugfix together with a unit test that will prevent the bug reoccurring. I'm not sure though whether _perfcheck is a feature that needs to be fixed (is it user facing?) or dead code that needs to be removed or a test that needs to move to the test module.

Any idea what it's for?

Copy link
Contributor

@rhettingerrhettingerMay 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't user facing and it isn't dead. We sometime have code the is just for us in a main section. For example, the random module also has some performance measurement code and example distributions — we use that sometimes while maintaining the module.

Copy link
Member

@iritkatrieliritkatrielMay 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it’s useful then it needs a test.

(And it would probably be even more useful as a pyperformance benchmark, where it would be tracked on a regular basis.)

Copy link
Contributor

@itaisteinherzitaisteinherzMay 10, 2022•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd write a sanity test which verifies that $ python -m pprint works. More unit tests could then be added in the future if needed, but a simple sanity test is enough for now.

From what I can tell, that is what didn't work for @ArturKhuziakhmetov when they opened #92546, and it makes sense to test. It is surely user-facing, and should work.

Copy link
Contributor

@itaisteinherzitaisteinherzMay 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: I just noticed I missed some of @rhettinger's comments. Since this is undocumented functionality, refraining from testing it may make sense. However, seeing broken code shipped in production builds raises a red flag for me.

Copy link
Member

@merwokmerwokMay 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the production code is really the functions that people import from the pprint module, not the unofficial script functionality.

t2 = time.perf_counter()
p.pformat(object)
t3 = time.perf_counter()
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2022-05-09-17-31-30.gh-issue-92546.1hbeQQ.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
Fix invalid :meth:`PrettyPrinter._safe_repr` call when :mod:`pprint` is
executed as a script.