-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-102615: Use list
instead of tuple
in repr
of paramspec
#102637
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
Conversation
sobolevn
commented
Mar 13, 2023
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: Representation of ParamSpecs at runtime compared to Callable #102615
I'm -1 on this fix. There is a bigger underlying issue. See #102615 (comment) |
I have no idea what to do here, I defer to @Fidget-Spinner. |
I'm now +1 on this approach after Nikita reminded me that ParamSpec intentionally has edge cases that we cant fix. |
This PR should be reviewed after #102681 |
@AlexWaygood you can review this whenever you have the time. |
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.
@AlexWaygood you can review this whenever you have the time.
Time, the most precious commodity of our age! Here's a review for the tests and docs :)
Misc/NEWS.d/next/Library/2023-03-13-12-05-55.gh-issue-102615.NcA_ZL.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
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.
Thanks! LGTM, at least as a short-term fix.
I don't think we should backport this, as it could easily break people's doctests. Let me know if you disagree :)
TsP[int, str, list[int], []]: "TsP[int, str, list[int], []]", | ||
TsP[int, [str, list[int]]]: "TsP[int, [str, list[int]]]", | ||
|
||
# These lines are just too long to fit: |
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.
😆
@sobolevn, should we make the same changes we made here to cpython/Lib/_collections_abc.py Line 515 in 51d693c
|
In fact... I wonder if we should just import |
@AlexWaygood I thought about making the similar change in Right now users can define their own classes with Any examples where this might be useful? I also think that sharing implementation details (functions starting with |
Okay, those are all good points. I think it might be nice at least to update the docstring of |
I will! |
…python#102637) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
…python#102637) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>