Skip to content

gh-106789: avoid importing pprint from sysconfig #106790

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

Jump to bottom
Merged
iritkatriel merged 6 commits into from
Jul 17, 2023
Merged

gh-106789: avoid importing pprint from sysconfig #106790

iritkatriel merged 6 commits into from
Jul 17, 2023

Conversation

@iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jul 15, 2023 •
edited by bedevere-bot
Loading

@iritkatriel iritkatriel requested a review from FFY00 as a code owner July 15, 2023 22:34
@bedevere-bot bedevere-bot mentioned this pull request Jul 15, 2023
Closed
@iritkatriel iritkatriel requested a review from gvanrossum July 15, 2023 22:34
gvanrossum
gvanrossum approved these changes Jul 15, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Too bad about the limerick. Maybe we can ask ChatGPT to write the commit message as a limerick. :-)

FFY00
FFY00 approved these changes Jul 15, 2023
View reviewed changes
Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

AFAICT repr for strings should result in (mostly) valid Python code representing the string in question, so I believe this patch is correct. At least, any edge case we could run into with repr we would likely run into with pprint anyway.

@gvanrossum
Copy link
Member

gvanrossum commented Jul 15, 2023

I don’t think there are any cases where repr() is wrong, are there?

@FFY00
Copy link
Member

FFY00 commented Jul 15, 2023 •
edited
Loading

I don't think so either, but I have seen too many edge cases in situations like this, so am not confident saying there aren't any here. I'm fairly confident that this change is okay, though.

methane
methane reviewed Jul 16, 2023
View reviewed changes
@iritkatriel iritkatriel added the stdlib Standard Library Python modules in the Lib/ directory label Jul 16, 2023
@iritkatriel iritkatriel merged commit 5ecedbd into python:main Jul 17, 2023
@iritkatriel iritkatriel deleted the opcode branch July 25, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methane methane methane approved these changes

@gvanrossum gvanrossum gvanrossum approved these changes

@FFY00 FFY00 FFY00 approved these changes

Assignees

No one assigned

Labels

stdlib Standard Library Python modules in the Lib/ directory

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sysconfig imports pprint which imports the world

5 participants

@iritkatriel @gvanrossum @FFY00 @methane @bedevere-bot