Skip to content

gh-127011: Add __str__ and __repr__ to ConfigParser #132966

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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

Agent-Hellboy
Copy link
Contributor

@Agent-Hellboy Agent-Hellboy commented Apr 25, 2025

Agent-Hellboy and others added 7 commits April 26, 2025 00:56
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
…pem5z.rst

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@StanFromIreland
Copy link
Contributor

You can batch apply suggestions from the Files Changed tab, please do so.

Agent-Hellboy and others added 3 commits April 26, 2025 01:10
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Comment on lines +1083 to +1085
return (f"<{self.__class__.__name__}("
f"params={init_params}, "
f"state={state_summary})>")
Copy link
Member

Choose a reason for hiding this comment

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

We could use = f-strings:

return f"<{self.__class__.__name__}({params=}, {state=})>"

but this requires the variables to be named "params" and "state" instead.

Copy link
Contributor Author

@Agent-Hellboy Agent-Hellboy Apr 25, 2025

Choose a reason for hiding this comment

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

I can change if you want but init_params and state_summary are more descriptive for those who are reading the code.

Copy link
Member

Choose a reason for hiding this comment

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

Mmh, yes. Let's leave keep it that way (I'll let Jaraco have the final say)

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

It looks good but I'll let @jaraco have the final say

Comment on lines +1083 to +1085
return (f"<{self.__class__.__name__}("
f"params={init_params}, "
f"state={state_summary})>")
Copy link
Member

Choose a reason for hiding this comment

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

Mmh, yes. Let's leave keep it that way (I'll let Jaraco have the final say)

Agent-Hellboy and others added 2 commits April 26, 2025 15:24
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@StanFromIreland
Copy link
Contributor

You do not need to click the button to update the branch, if it’s needed a core dev will run it before merging.

Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

looks good so far

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants