-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-102721: Improve coverage of _collections_abc._CallableGenericAlias
#102722
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -481,15 +481,8 @@ def __getitem__(self, item): | |
# rather than the default types.GenericAlias object. Most of the | ||
# code is copied from typing's _GenericAlias and the builtin | ||
# types.GenericAlias. | ||
|
||
if not isinstance(item, tuple): | ||
item = (item,) | ||
# A special case in PEP 612 where if X = Callable[P, int], | ||
# then X[int, str] == X[[int, str]]. | ||
if (len(self.__parameters__) == 1 | ||
and _is_param_expr(self.__parameters__[0]) | ||
and item and not _is_param_expr(item[0])): | ||
item = (item,) | ||
Comment on lines
-487
to
-492
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes me slightly nervous, but I can't find any changes in behaviour from removing this code. @Fidget-Spinner and @serhiy-storchaka, can either of you think of anything bad that could happen if we remove this code? The whole test suite passes with it removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, me too. Let's wait for others to comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes me nervous too. In case you're looking for an example that triggers this path, here's one: from typing import ParamSpec
P = ParamSpec("P")
from collections.abc import Callable
C = Callable[[P], int]
print(C[str]) However I agree that if I just comment this out, the output is the same. It's possible that @serhiy-storchaka fixed the issue in the C code (the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I found examples that triggered this code path, but for all of the examples I found, I couldn't find any differences in behaviour with this code deleted (repr, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Someone could trace through the C code that gets called and see if it does the right thing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like It unpacks nested tuples to a flat format, so - this code is not needed anymore. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't suppose the pure-Python code could be useful for PyPy, could it? (Don't see how it could, just throwing that out there) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, the only other iterperter I worked on is |
||
|
||
new_args = super().__getitem__(item).__args__ | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.