GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-88773: Added teleport method to Turtle library by liam-gersten · Pull Request #103974 · python/cpython

Conversation

@liam-gersten

@liam-gersten liam-gersten commented Apr 28, 2023 •
edited by gpshead
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fully implemented and tested a teleport method as described by the issue. This method behaves differently from the goto (or setpos) method, and can be especially helpful when creating and filling multiple separate objects.

Implementation:

There already exists a goto method, aliased as setpos | setposition | goto. It basically moves the turtle to the given position, but if the pen is down, it draws a line along the path. Additionally, the move is not instant, and you can see it go along the path to get to the destination. Users of the library may find themselves raising the pen, calling the method, and then lowering it so that a line doesn’t get drawn. Another issue with using goto for this is that regardless of whether the pen is raised, an "imaginary" line is still drawn from the current position to the destination. When filling is enabled, this invisible line acts as a boundary for the fill color. If one were creating multiple separate objects with filling enabled, they would need to run the exact code in the teleport method in order to fill them separately as desired. The following is a demo of the difference between goto and teleport for filling multiple objects.

Justification:

One might think we should just update the goto method to behave this way with a flag like teleport=True. However, all of the pen controlling methods are in one class, TPen, and all of the visual and position controlling methods are in another, TNavigator. The primary turtle class, RawTurtle(TPen, TNavigator) inherits methods from both of these, and since teleport needs access to methods and attributes in both of these classes, this is where it should be written. While it would be possible to give goto access to the TPen class via inheritance, this would be a messy refactor for what should be a distinct method.

Resolves #88773

  • Issue: Teleport method for turtle class #88773

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
liam-gersten added 3 commits April 27, 2023 17:42
@liam-gersten
Working teleport method
a51d835
@liam-gersten
Handle filling with teleport method
7986926
@liam-gersten
Teleport method documentation
1054b34
@bedevere-bot

This comment was marked as resolved.

Sign in to view
@bedevere-bot bedevere-bot mentioned this pull request Apr 28, 2023
Teleport method for turtle class #88773
Closed
@bedevere-bot bedevere-bot added the awaiting review label Apr 28, 2023
@liam-gersten
Merge branch 'main' into TurtleTeleportMethod
af7c1c6
@ghost

ghost commented Apr 28, 2023 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@arhadthedev arhadthedev added stdlib Standard Library Python modules in the Lib/ directory topic-tkinter labels Apr 28, 2023
blurb-it Bot and others added 2 commits April 28, 2023 18:04
@blurb-it
📜🤖 Added by blurb_it.
5871c2c
@liam-gersten
Fixed NEWS entry
494f478
@terryjreedy terryjreedy removed the topic-tkinter label Apr 28, 2023
@gpshead gpshead self-assigned this Apr 28, 2023
@gpshead gpshead added the type-feature A feature request or enhancement label Apr 28, 2023
terryjreedy
terryjreedy requested changes Apr 28, 2023
View reviewed changes

@terryjreedy terryjreedy left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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 cannot comment on the desirability of the addition until I have time to test and verify justification assertion. Not now.
Nor have I reviewed the new tkinter code.

Blurb needs expansion. I will commit my proposal and you can propose further revision.

Doc and test additions are needed before merge.

Note: Topics are for issues. Core developers get notified of PRs by signing up as 'code owner' of specific files. Besides which, this does not touch tkinter.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

liam-gersten reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Misc/NEWS.d/next/Library/2023-04-28-18-04-23.gh-issue-88773.xXCNJw.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels Apr 28, 2023
@bedevere-bot

bedevere-bot commented Apr 28, 2023

Copy link
Copy Markdown

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 phrase I 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.

liam-gersten reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@terryjreedy
Update Misc/NEWS.d/next/Library/2023-04-28-18-04-23.gh-issue-88773.xX…
2fd8eda
…CNJw.rst
gpshead
gpshead requested changes Apr 28, 2023
View reviewed changes

@gpshead gpshead left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

Please also document this in Doc/library/turtle.rst. Be sure to include a .. versionadded: 3.12 annotation on the new teleport method documentation in there.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

liam-gersten reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/turtle.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@python python deleted a comment from bedevere-bot Apr 28, 2023
liam-gersten added 3 commits April 28, 2023 15:18
@liam-gersten
Merge branch 'main' into TurtleTeleportMethod
e89549e
@liam-gersten
Made new coord assignment more concise
ba2920e
@liam-gersten
Merge branch 'TurtleTeleportMethod' of https://github.com/liam-gerste…
01dd697
…n/cpython into TurtleTeleportMethod
@Fidget-Spinner

Fidget-Spinner commented Apr 28, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Terry and Gregory, thank you for taking the time to review this PR.

If you're wondering where to add tests, they are found in /Lib/test/. In this case, it is test_turtle.py. You can run the test with ./python -m test test_turtle. Read the devguide here to learn more https://devguide.python.org/testing/run-write-tests/ .

Documentation help can be found here https://devguide.python.org/documentation/ .

I also sent you an email to discuss CPython contribution. Please take a look when you have the time. Thanks!

liam-gersten, gpshead, and erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@liam-gersten
Teleport documentation in Doc/library/turtle.rst
f8a9681
hugovk
hugovk reviewed Apr 28, 2023
View reviewed changes
Comment thread Doc/library/turtle.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/turtle.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Misc/NEWS.d/next/Library/2023-04-28-18-04-23.gh-issue-88773.xXCNJw.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

liam-gersten and others added 2 commits April 28, 2023 16:10
@liam-gersten @hugovk
Update Doc/library/turtle.rst
7cec9b6
Co-authored-by: Hugo van Kemenade <[email protected]>
@liam-gersten @hugovk
Update Misc/NEWS.d/next/Library/2023-04-28-18-04-23.gh-issue-88773.xX…
eec5411
…CNJw.rst

Co-authored-by: Hugo van Kemenade <[email protected]>
@bedevere-bot bedevere-bot added the awaiting change review label Apr 28, 2023
@bedevere-bot

bedevere-bot commented Apr 28, 2023

Copy link
Copy Markdown

Thanks for making the requested changes!

@terryjreedy, @gpshead: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot
bedevere-bot requested review from gpshead and terryjreedy April 28, 2023 20:48
liam-gersten added 2 commits April 28, 2023 16:50
@liam-gersten
Made teleport appear in alphabetical order
ae01a3e
@liam-gersten
Merge branch 'TurtleTeleportMethod' of https://github.com/liam-gerste…
43f8a69
…n/cpython into TurtleTeleportMethod
liam-gersten

This comment was marked as outdated.

Sign in to view

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gpshead added 4 commits April 28, 2023 17:45
@gpshead
Attempt to fix the doctests.
07ae3da
stamp ids are global an incrementing, do not expose them as doctest constants.
@gpshead
Mention fill_gap in the teleport prototype.
d648f52
@gpshead
Make fill_gap a keyword only argument.
2baaf85
@gpshead
Align docs with fill_gap being keyword only.
203a0f3
gpshead
gpshead reviewed Apr 29, 2023
View reviewed changes
Comment thread Lib/test/test_turtle.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

gpshead
gpshead reviewed Apr 29, 2023
View reviewed changes
Comment thread Lib/test/test_turtle.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Apr 29, 2023

Copy link
Copy Markdown
Member

I wish the turtle module was a 3rd party package, but since it isn't, and is actively used, I think this is a good contribution. I hope @gpshead will come through with his promise of review of the PR. (I didn't see anything objectionable on a quick skim.)

gpshead reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

liam-gersten added 2 commits April 30, 2023 12:53
@liam-gersten
Testing changes for fill_gap arg and assertFalse
dac054e
@liam-gersten
Merge branch 'main' into TurtleTeleportMethod
9a38e8a
@liam-gersten

liam-gersten commented Apr 30, 2023

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot

bedevere-bot commented Apr 30, 2023

Copy link
Copy Markdown

Thanks for making the requested changes!

@gpshead, @terryjreedy: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot
bedevere-bot requested a review from gpshead April 30, 2023 16:56
@liam-gersten
Merge branch 'main' into TurtleTeleportMethod
fc880c8
gpshead
gpshead approved these changes Apr 30, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Apr 30, 2023
@gpshead
gpshead merged commit 74a2b79 into python:main Apr 30, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 30, 2023
@hugovk hugovk mentioned this pull request May 1, 2023
gh-65276: Fixed Turtle library circle method bug #104022
Open
carljm added a commit to carljm/cpython that referenced this pull request May 1, 2023
@carljm
Merge branch 'main' into supermon
159b2aa
* main: (26 commits)
  pythongh-104028: Reduce object creation while calling callback function from gc (pythongh-104030)
  pythongh-104036: Fix direct invocation of test_typing (python#104037)
  pythongh-102213: Optimize the performance of `__getattr__` (pythonGH-103761)
  pythongh-103895: Improve how invalid `Exception.__notes__` are displayed (python#103897)
  Adjust expression from `==` to `!=` in alignment with the meaning of the paragraph. (pythonGH-104021)
  pythongh-88496: Fix IDLE test hang on macOS (python#104025)
  Improve int test coverage (python#104024)
  pythongh-88773: Added teleport method to Turtle library (python#103974)
  pythongh-104015: Fix direct invocation of `test_dataclasses` (python#104017)
  pythongh-104012: Ensure test_calendar.CalendarTestCase.test_deprecation_warning consistently passes (python#104014)
  pythongh-103977: compile re expressions in platform.py only if required (python#103981)
  pythongh-98003: Inline call frames for CALL_FUNCTION_EX (pythonGH-98004)
  Replace Netlify with Read the Docs build previews (python#103843)
  Update name in acknowledgements and add mailmap (python#103696)
  pythongh-82054: allow test runner to split test_asyncio to execute in parallel by sharding. (python#103927)
  Remove non-existing tools from Sundry skiplist (python#103991)
  pythongh-103793: Defer formatting task name (python#103767)
  pythongh-87092: change assembler to use instruction sequence instead of CFG (python#103933)
  pythongh-103636: issue warning for deprecated calendar constants (python#103833)
  Various small fixes to dis docs (python#103923)
  ...
@hugovk hugovk mentioned this pull request May 8, 2023
Trim trailing whitespace and test on CI #104275
Merged
This was referenced Aug 9, 2023
turtle.teleport has incorrect-ish signature #107805
Closed
gh-107805: Fix signatures of module-level generated functions in turtle #107807
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk left review comments
@gpshead gpshead gpshead approved these changes
@terryjreedy terryjreedy Awaiting requested review from terryjreedy

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Teleport method for turtle class

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@liam-gersten @bedevere-bot @Fidget-Spinner @gvanrossum @gpshead @hugovk @terryjreedy @arhadthedev
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.