Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-104050: Annotate Argument Clinic parameter permutation helpers#106431
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-104050: Annotate Argument Clinic parameter permutation helpers#106431
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
erlend-aasland commentedJul 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
AlexWaygood commentedJul 4, 2023
:( I'm sorry to hear that. There's definitely a lot of concepts to grapple with, I'll fully concede. Wehave uncovered at least two real bugs along the way, though! |
erlend-aasland commentedJul 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
No worries, I'm having fun (slowly learning)! And I think this whole project will end up increasing the readability and maintainability of clinic.py. I'm just frustrated when I can't figure out new stuff, nothing else 😄 |
erlend-aasland commentedJul 4, 2023
Also, once again, thanks for helping reviewing and explaining! |
erlend-aasland commentedJul 4, 2023
So, here's what mypy complains about on my computer: Tools/clinic/clinic.py:536: error: Incompatible types in assignment (expression has type "list[Iterable[Parameter] | Parameter]", variable has type "list[Iterable[Parameter]]") [assignment]Tools/clinic/clinic.py:553: error: Argument 1 to "extend" of "list" has incompatible type "Iterable[Parameter]"; expected "Iterable[Iterable[Parameter]]" [arg-type]Tools/clinic/clinic.py:584: error: Argument 1 to "append" of "list" has incompatible type "tuple[Iterable[Parameter], ...]"; expected "Iterable[Parameter]" [arg-type]Tools/clinic/clinic.py:586: error: Argument "key" to "sort" of "list" has incompatible type "Callable[[Sized], int]"; expected "Callable[[Iterable[Parameter]], SupportsDunderLT[Any] | SupportsDunderGT[Any]]" [arg-type] Is mypy fooled by the |
Uh oh!
There was an error while loading.Please reload this page.
AlexWaygood commentedJul 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Humm, I wrote you a small essay above there, but I'm still working on getting mypy to actuallypass with my suggestions... I'll get back to you :) |
AlexWaygood commentedJul 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Okay! Here's a diff that makes mypy happy, and Ithink it has the added virtue of being correct. How does it look to you? Details--- a/Tools/clinic/clinic.py+++ b/Tools/clinic/clinic.py@@ -28,7 +28,7 @@ import textwrap import traceback-from collections.abc import Callable+from collections.abc import Callable, Iterable, Iterator, Sequence from types import FunctionType, NoneType from typing import ( Any,@@ -516,7 +516,11 @@ class PythonLanguage(Language): checksum_line = "#/*[{dsl_name} end generated code: {arguments}]*/"-def permute_left_option_groups(l):+ParamGroupSequence = Sequence[Iterable["Parameter"]]+ParamGroupTuple = tuple["Parameter", ...]+++def permute_left_option_groups(l: ParamGroupSequence) -> Iterator[ParamGroupTuple]: """ Given [(1,), (2,), (3,)], should yield: ()@@ -525,13 +529,13 @@ def permute_left_option_groups(l): (1, 2, 3) """ yield tuple()- accumulator = []+ accumulator: list[Parameter] = [] for group in reversed(l): accumulator = list(group) + accumulator yield tuple(accumulator)-def permute_right_option_groups(l):+def permute_right_option_groups(l: ParamGroupSequence) -> Iterator[ParamGroupTuple]: """ Given [(1,), (2,), (3,)], should yield: ()@@ -540,13 +544,17 @@ def permute_right_option_groups(l): (1, 2, 3) """ yield tuple()- accumulator = []+ accumulator: list[Parameter] = [] for group in l: accumulator.extend(group) yield tuple(accumulator)-def permute_optional_groups(left, required, right):+def permute_optional_groups(+ left: ParamGroupSequence,+ required: Iterable[Parameter],+ right: ParamGroupSequence+) -> tuple[ParamGroupTuple, ...]: """ Generator function that computes the set of acceptable argument lists for the provided iterables of@@ -561,7 +569,7 @@ def permute_optional_groups(left, required, right): if left: raise ValueError("required is empty but left is not")- accumulator = []+ accumulator: list[ParamGroupTuple] = [] counts = set() for r in permute_right_option_groups(right): for l in permute_left_option_groups(left): |
AlexWaygood commentedJul 4, 2023
Argh, the names of those type aliases are quite bad actually. (Again, this shows the peril of type aliases!) Maybe this instead: --- a/Tools/clinic/clinic.py+++ b/Tools/clinic/clinic.py@@ -28,7 +28,7 @@ import textwrap import traceback-from collections.abc import Callable+from collections.abc import Callable, Iterable, Iterator, Sequence from types import FunctionType, NoneType from typing import ( Any,@@ -516,7 +516,11 @@ class PythonLanguage(Language): checksum_line = "#/*[{dsl_name} end generated code: {arguments}]*/"-def permute_left_option_groups(l):+SequenceOfParameterGroups = Sequence[Iterable["Parameter"]]+ParameterTuple = tuple["Parameter", ...]+++def permute_left_option_groups(l: SequenceOfParameterGroups) -> Iterator[ParameterTuple]: """ Given [(1,), (2,), (3,)], should yield: ()@@ -525,13 +529,13 @@ def permute_left_option_groups(l): (1, 2, 3) """ yield tuple()- accumulator = []+ accumulator: list[Parameter] = [] for group in reversed(l): accumulator = list(group) + accumulator yield tuple(accumulator)-def permute_right_option_groups(l):+def permute_right_option_groups(l: SequenceOfParameterGroups) -> Iterator[ParameterTuple]: """ Given [(1,), (2,), (3,)], should yield: ()@@ -540,13 +544,17 @@ def permute_right_option_groups(l): (1, 2, 3) """ yield tuple()- accumulator = []+ accumulator: list[Parameter] = [] for group in l: accumulator.extend(group) yield tuple(accumulator)-def permute_optional_groups(left, required, right):+def permute_optional_groups(+ left: SequenceOfParameterGroups,+ required: Iterable[Parameter],+ right: SequenceOfParameterGroups+) -> tuple[ParameterTuple, ...]: """ Generator function that computes the set of acceptable argument lists for the provided iterables of@@ -561,7 +569,7 @@ def permute_optional_groups(left, required, right): if left: raise ValueError("required is empty but left is not")- accumulator = []+ accumulator: list[ParameterTuple] = [] counts = set() for r in permute_right_option_groups(right): for l in permute_left_option_groups(left): |
- Use a single alias: ParamTuple- Ditch generators, use Iterator- Use Sequence iso. layers of Iterable/Reversible/etc.
erlend-aasland commentedJul 4, 2023
I did a variant with only one (IMO slightly better named) alias ;) Thank you so much! I'm never found out what the |
This comment was marked as outdated.
This comment was marked as outdated.
Uh oh!
There was an error while loading.Please reload this page.
AlexWaygood commentedJul 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The |
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
AlexWaygood left a comment
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.
Now we're there ;)
This was a very tricky one!
* main: (39 commits)pythongh-102542 Remove unused bytes object and bytes slicing (python#106433) Clarify state of CancelledError in doc (python#106453)pythongh-64595: Fix regression in file write logic in Argument Clinic (python#106449)pythongh-104683: Rename Lib/test/clinic.test as Lib/test/clinic.test.c (python#106443) tp_flags docs: fix indentation (python#106420)pythongh-104050: Partially annotate Argument Clinic CLanguage class (python#106437)pythongh-106368: Add tests for formatting helpers in Argument Clinic (python#106415)pythongh-104050: Annotate Argument Clinic parameter permutation helpers (python#106431)pythongh-104050: Annotate toplevel functions in clinic.py (python#106435)pythongh-106320: Fix specialize.c compilation by including pycore_pylifecycle.h (python#106434) Add some codeowners for `Tools/clinic/` (python#106430)pythongh-106217: Truncate the issue body size of `new-bugs-announce-notifier` (python#106423)pythongh-61215: Rename `wait_until_any_call` to `wait_until_any_call_with` (python#106414)pythongh-106162: array: suppress warning in test_array (python#106404)pythongh-106320: Remove _PyInterpreterState_HasFeature() (python#106425)pythonGH-106360: Support very basic superblock introspection (python#106422)pythongh-106406: Fix _Py_IsInterpreterFinalizing() in _winapi.c (python#106408)pythongh-106396: Special-case empty format spec to gen empty JoinedStr node (python#106401)pythongh-106368: Add tests for permutation helpers in Argument Clinic (python#106407)pythonGH-106008: Fix refleak when peepholing `None` comparisons (python#106367) ...
Uh oh!
There was an error while loading.Please reload this page.