Skip to content

gh-65772: Clean-up turtle module #104218

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

Merged
merged 5 commits into from
May 6, 2023
Merged

gh-65772: Clean-up turtle module #104218

merged 5 commits into from
May 6, 2023

Conversation

terryjreedy
Copy link
Member

@terryjreedy terryjreedy commented May 5, 2023

  • Remove the unused, private, and undocumented name _ver and the commented-out print call.

  • Don't add math functions to __all__. Beginners should learn to import math to access them.

  • Gregor Lindel, who wrote this version of turtle, dropped plans to implement turtle on another toolkit at least a decade ago. Drop _dot code preparing for this, but add a hint comment.

  • _Screen is meant to be a singleton class. To enforce that, it needs either a __new__ that returns the singleton or else...raise in __iter__. Merely removing the if clauses as suggested might break something if a user were to call _Screen directly. Leave the code alone until a problem is evident.

  • Turtledemo injects into _Screen both _root and _canvas, configured as it needs them to be. Making _canvas an __init__ option would require skipping some but not all of the lines under 'if _Screen._canvas is None:`. Leave working code alone.

* Remove the unused, private, and undocumented name `_ver` and
the commented-out `print` call.

* Don't add math functions to `__all__`.  Beginners should learn
to `import math` to access them.

* Gregor Lindel, who wrote this version of turtle, dropped plans
to implement turtle on another toolkit at least a decade ago.
Drop `_dot` code preparing for this, but add a hint comment.

* `_Screen` is meant to be a singleton class.  To enforce that,
it needs either a `__new__` that returns the singleton or
`else...raise` in `__iter__`.  Merely removing the `if` clauses
as suggested might break something if a user were to call `_Screen`
directly.  Leave the code alone until a problem is evident.

* Turtledemo injects into _Screen both _root and _canvas,
configured as it needs them to be.  Making _canvas an `__init__`
option would require skipping some but not all of the lines under
'if _Screen._canvas is None:`.  Leave working code alone.
@rhettinger rhettinger removed their request for review May 6, 2023 01:45
@terryjreedy
Copy link
Member Author

terryjreedy commented May 6, 2023

I manually verified that def dot works after removal of if hasattr(self.screen, "_dot"): ... block and subsequent dedent of else: block. I plan to merge tomorrow.

Replace `self._canvas` and `self.scanvas`, both bound to `canvas`,
with `self.canvas, which is accessed in other methods.
Replace `_s_` with `screen` and `_s_._canvas` with `canvas`.
Add a comment explaining the unorthodox use of
function turtle.Screen and singleton class turtle._Screen.
@terryjreedy terryjreedy merged commit e407661 into python:main May 6, 2023
@terryjreedy terryjreedy deleted the turtle branch May 6, 2023 15:05
@python python deleted a comment from bedevere-bot May 6, 2023
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
* Remove the unused, private, and undocumented name `_ver` and
the commented-out `print` call.

* Don't add math functions to `__all__`.  Beginners should learn
to `import math` to access them.

* Gregor Lindel, who wrote this version of turtle, dropped plans
to implement turtle on another toolkit at least a decade ago.
Drop `_dot` code preparing for this, but add a hint comment.

* `_Screen` is meant to be a singleton class.  To enforce that,
it needs either a `__new__` that returns the singleton or
`else...raise` in `__iter__`.  Merely removing the `if` clauses
as suggested might break something if a user were to call `_Screen`
directly.  Leave the code alone until a problem is evident.

* Turtledemo injects into _Screen both _root and _canvas,
configured as it needs them to be.  Making _canvas an `__init__`
option would require skipping some but not all of the lines under
'if _Screen._canvas is None:`.  Leave working code alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants