Skip to content

gh-102615: Use list instead of tuple in repr of paramspec - #102637

Merged
AlexWaygood merged 5 commits into
python:mainfrom
sobolevn:issue-102615
Mar 15, 2023
Merged

gh-102615: Use list instead of tuple in repr of paramspec#102637
AlexWaygood merged 5 commits into
python:mainfrom
sobolevn:issue-102615

Conversation

@sobolevn

@sobolevn sobolevn commented Mar 13, 2023

Copy link
Copy Markdown
Member
@Fidget-Spinner

Copy link
Copy Markdown
Member

I'm -1 on this fix. There is a bigger underlying issue. See #102615 (comment)

@gvanrossum

Copy link
Copy Markdown
Member

I have no idea what to do here, I defer to @Fidget-Spinner.

@Fidget-Spinner

Copy link
Copy Markdown
Member

I'm now +1 on this approach after Nikita reminded me that ParamSpec intentionally has edge cases that we cant fix.

@sobolevn

Copy link
Copy Markdown
Member Author

This PR should be reviewed after #102681
I think I might need to modify it afterwards :)

@sobolevn
sobolevn marked this pull request as draft March 14, 2023 07:20
@sobolevn
sobolevn marked this pull request as ready for review March 14, 2023 14:41
@sobolevn

Copy link
Copy Markdown
Member Author

@AlexWaygood you can review this whenever you have the time.

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

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 :)

Comment thread Lib/test/test_typing.py Outdated
Comment thread Misc/NEWS.d/next/Library/2023-03-13-12-05-55.gh-issue-102615.NcA_ZL.rst Outdated
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@AlexWaygood
AlexWaygood self-requested a review March 14, 2023 15:11
Comment thread Lib/test/test_typing.py Outdated
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Comment thread Lib/test/test_typing.py Outdated
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@gvanrossum
gvanrossum removed their request for review March 14, 2023 17:21
Comment thread Lib/test/test_typing.py
Comment thread Lib/typing.py
Comment thread Lib/typing.py

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

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 :)

Comment thread Lib/test/test_typing.py
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:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😆

@AlexWaygood

Copy link
Copy Markdown
Member

@sobolevn, should we make the same changes we made here to _collections_abc._type_repr? The docstring of that function indicates it is meant to have identical functionality to typing._type_repr.

def _type_repr(obj):

@AlexWaygood

AlexWaygood commented Mar 16, 2023

Copy link
Copy Markdown
Member

In fact... I wonder if we should just import _type_repr from _collections_abc in typing, instead of duplicating it between the two modules. It's obvious why _collections_abc shouldn't depend on typing, but I don't see any reason why typing shouldn't depend on _collections_abc. We already have import collections.abc at the top of the file.

@sobolevn

sobolevn commented Mar 16, 2023

Copy link
Copy Markdown
Member Author

@AlexWaygood I thought about making the similar change in _collections_abc._type_repr. But, what is the use-case?

Right now users can define their own classes with ParamSpec only using typing.Generic. So, no use for _collections_abc.
As far as I know, the only std type that supports it is _collections_abc.Callable, but it already supports the correct repr.

Any examples where this might be useful?

I also think that sharing implementation details (functions starting with _) creates high coupling and implicit contracts, which is bad thing.

@AlexWaygood

AlexWaygood commented Mar 16, 2023

Copy link
Copy Markdown
Member

Okay, those are all good points. I think it might be nice at least to update the docstring of typing._type_repr to remind us that there's a function with very similar functionality in _collections_abc, though. What do you think?

@sobolevn

Copy link
Copy Markdown
Member Author

I will!

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
…python#102637)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
…python#102637)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants