Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-107805: Fix signatures of module-level generated functions inturtle#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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
gpshead merged 7 commits intopython:mainfromsobolevn:issue-107805
Sep 1, 2023

Conversation

@sobolevn
Copy link
Member

@sobolevnsobolevn commentedAug 9, 2023•
edited by bedevere-bot
Loading

Now we are usinginspect.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

@sobolevn
Copy link
MemberAuthor

I think that this can go into rc2, because it fixes a bug which makesturtle.teleport unusable.

Eclips4 and gpshead reacted with thumbs up emoji

@gpsheadgpshead added type-bugAn unexpected behavior, bug, or error needs backport to 3.12only security fixes release-blocker labelsAug 10, 2023
@gpsheadgpshead self-assigned thisAug 10, 2023
Copy link
Member

@gpsheadgpshead left a 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.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

'pen':'(pen=None, **pendict)',
}

fornameinturtle.__all__:
Copy link
Member

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 overknown_signatures.items() rather thanturtle.__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.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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:

  • Callteleport() without arguments.
  • Callteleport() with three positional arguments (it should fail).

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@sobolevn
Copy link
MemberAuthor

I've addressed@gpshead comments about the test case, but I don't think that I can actually test these functions:

  1. Nothing similar is tested intest_turtle, it simply does not have anyTurtle class tests at all
  2. I don't know anything abouttkinter
  3. I don't really think that we should add test foundation for it in this specific PR
  4. inspect works with real python function signature, so it is at least something

If anyone wants to contribute sometkinter-based tests forTurtle class, I would be happy to includeteleport test there 👍

gpshead reacted with thumbs up emoji

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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.

Copy link
Member

@gpsheadgpshead left a 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.

@gpsheadgpshead merged commit044b8b3 intopython:mainSep 1, 2023
@miss-islington
Copy link
Contributor

Thanks@sobolevn for the PR, and@gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestSep 1, 2023
…n `turtle` (pythonGH-107807)(cherry picked from commit044b8b3)Co-authored-by: Nikita Sobolev <mail@sobolevn.me>Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@bedevere-bot
Copy link

GH-108749 is a backport of this pull request to the3.12 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.12only security fixes labelSep 1, 2023
Yhg1s pushed a commit that referenced this pull requestSep 1, 2023
…in `turtle` (GH-107807) (#108749)gh-107805: Fix signatures of module-level generated functions in `turtle` (GH-107807)(cherry picked from commit044b8b3)Co-authored-by: Nikita Sobolev <mail@sobolevn.me>Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@adorilsonadorilson mentioned this pull requestJun 28, 2025
21 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@gpsheadgpsheadgpshead approved these changes

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

@terryjreedyterryjreedyAwaiting requested review from terryjreedy

Assignees

@gpsheadgpshead

Labels

release-blockertype-bugAn unexpected behavior, bug, or error

Projects

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sobolevn@bedevere-bot@miss-islington@gpshead@serhiy-storchaka@AlexWaygood
[8]ページ先頭
©2009-2025 Movatter.jp