-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
gh-107805: Fix signatures of module-level generated functions in turtle
#107807
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
I think that this can go into rc2, because it fixes a bug which makes |
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.
likely good, but i'll take a closer look tomorrow. one suggestion added.
Lib/test/test_turtle.py
Outdated
'pen': '(pen=None, **pendict)', | ||
} | ||
|
||
for name in turtle.__all__: |
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.
The purpose of this test is only to verify known signatures, correct?
In that case, looping over known_signatures.items()
rather than turtle.__all__
makes more sense. Otherwise a typo in a known_signatures name key would go silently unnoticed and we're otherwise creating subtests for all names in turtle even though most of them don't test anything.
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.
Well, I think that it would be nice to add also tests for the original issue:
- Call
teleport()
without arguments. - Call
teleport()
with three positional arguments (it should fail).
I've addressed @gpshead comments about the test case, but I don't think that I can actually test these functions:
If anyone wants to contribute some |
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.
Well, testing module-level turtle function is a non-trivial task.
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 manually tested that turtle.Turtle() appears to work and that Turtle.teleport works.
…n `turtle` (pythonGH-107807) (cherry picked from commit 044b8b3) Co-authored-by: Nikita Sobolev <[email protected]> Co-authored-by: Alex Waygood <[email protected]>
…in `turtle` (GH-107807) (#108749) gh-107805: Fix signatures of module-level generated functions in `turtle` (GH-107807) (cherry picked from commit 044b8b3) Co-authored-by: Nikita Sobolev <[email protected]> Co-authored-by: Alex Waygood <[email protected]>
Now we are using
inspect.signature
which works correct for all possible parameter types.There are no
/
params to test it, but it uses the same branch as regular pos-or-keyword params, so it should be fine.Related #103974
turtle.teleport
has incorrect-ish signature #107805