-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
base: main
Are you sure you want to change the base?
Conversation
Agent-Hellboy
commented
Apr 25, 2025
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: Add __str__ and __repr__ to ConfigParser for Better Debugging and Unit Testing #127011
Misc/NEWS.d/next/Library/2025-04-25-18-29-10.gh-issue-127011.Ipem5z.rst
Outdated
Show resolved
Hide resolved
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>
You can batch apply suggestions from the Files Changed tab, please do so. |
Misc/NEWS.d/next/Library/2025-04-25-18-29-10.gh-issue-127011.Ipem5z.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
return (f"<{self.__class__.__name__}(" | ||
f"params={init_params}, " | ||
f"state={state_summary})>") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this 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
return (f"<{self.__class__.__name__}(" | ||
f"params={init_params}, " | ||
f"state={state_summary})>") |
There was a problem hiding this comment.
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)
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
You do not need to click the button to update the branch, if it’s needed a core dev will run it before merging. |
There was a problem hiding this 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