Repository navigation
Clean up turtle.py code formatting #65772
Description
| BPO | 21573 |
|---|---|
| Nosy | @rhettinger, @terryjreedy, @ezio-melotti, @bitdancer, @AnishShah |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2014-05-25.01:48:23.909>
labels = ['type-feature']
title = 'Clean up turtle.py code formatting'
updated_at = <Date 2016-02-06.09:04:38.015>
user = 'https://bugs.python.org/jesstess'bugs.python.org fields:
activity = <Date 2016-02-06.09:04:38.015>
actor = 'anish.shah'
assignee = 'jesstess'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2014-05-25.01:48:23.909>
creator = 'jesstess'
dependencies = []
files = []
hgrepos = []
issue_num = 21573
keywords = ['gsoc']
message_count = 12.0
messages = ['219068', '219072', '219410', '219427', '219438', '219462', '219476', '219524', '219536', '219537', '219610', '223765']
nosy_count = 9.0
nosy_names = ['rhettinger', 'terry.reedy', 'gregorlingl', 'ezio.melotti', 'r.david.murray', 'jesstess', 'ingrid', 'Lita.Cho', 'anish.shah']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue21573'
versions = ['Python 3.5']
Activity
jesstess commentedon May 25, 2014
Lib/turtle.py has some code formatting issues. Let's clean them up to make the module easier to read as interns start working on it this summer. Specifically:
Run turtle.py through a PEP-8 checker and fix the issues that are reasonable to fix.
Run turtle.py through a linter like pyflakes and fix reasonable linter issues.
Examine commented out code, and either remove it or open a ticket if it represents an issue that should be fixed.
Examine # XXX comments, and either remove them if they are no longer applicable, or open tickets for them if they still represent bugs and then remove them.
LitaCho commentedon May 25, 2014
I'm claiming this ticket. Plan to work on it during my OPW internship.
terryjreedy commentedon May 30, 2014
I am glad to see this. Gregor has been inactive for several years, Except for Ned's recent patch for OSX, it has been years since there was a turtle-specific code patch (from hg revision history).
Questions: is there project link? are any of the mentors core developers, with commit rights? or would you need commits from someone like me?
I have read turtle.py and found that the multiple layers made some things hard to follow. To me, this was a bigger impediment than code formatting. You may want to develop an outline of the layer structure.
As you may know, Guido generally discourages pure code cleanups and prefers that they be done when the code is examined for other purposes. I personally think some changes are safe enough if verified by a second person.
I looked for and did not fine test/test_turtle. Did I miss something? Turtledemo is a partial substitute, but it might not exercise all turtle functions.
If you only apply cleanups to 3.5, subsequent bug fixes for all 3 versions will become much more difficult. The default commit process assumes that maintenance patches merge forward cleanly. I try to keep Idle code the same across versions as much as possible.
LitaCho commentedon May 30, 2014
Hi Terry,
I am not 100% sure. Let me ask Jessica, who is my mentor, and get back to you.
There is no project link to Turtle cleanup specifically. But I can definitely try to reorganize the code such that it flows better and it is easier to read. I can add that to the list.
I actually did not know that Guido discourages pure code cleanup. Should I try to find a feature to add? Currently, I am doing all the easy stuff, and making it PEP-8 compliant and work through linter. I am also trying to delete some of the commented out code.
I actually don't see the Turtle tests as well. Should creating unit tests for turtle.py be a separate ticket?
I am currently developing off of 3.4. I will try to run my patch through all the versions of Python 3 to make sure it is a clean fix.
terryjreedy commentedon May 31, 2014
When I re-read the top of the file, I remembered that turtle.py has unique issues in relation to code ownership. (I was once told that Gregor had to approve any substantive code change.) I asked about that and about code cleanups on pydev (thread 'updating turtle.py').
Re your message: I was suggesting 'mapping' the code, not re-structuring it.
Testing: A complete 'unit' test would test each function in each layer. A minimal 'unit' test should at least test each top-level function and check response on the canvas. Assuming that one can get to tk root and canvas, some things should be possible. But I don't know what introspection functions a canvas has. An alternative would be to replace the canvas with a mock-canvas with extra introspection added. Another alternative would be a human-verified test, a turtle script that systematically called every function and said that it was doing for a person to verify. "Line width: 1, 2, 3, 5, 8, 12 17, 30" (with a slight pause for each width).
Versions: at most 2.7, 3.4, 3.5. The 3.4 and 3.5 turtle code should be close to identical.
jesstess commentedon May 31, 2014
Terry, thank you for all the time you've been putting into the GSoC and OPW tickets.
https://wiki.python.org/moin/OPW/2014#Graphical_Python is a broad-strokes outline. As we get further into the internship we'll decide on areas of focus. I'm the main mentor. I don't have commit rights but would be reviewing most of the changes before putting them in the commit review stage.
You didn't miss anything. :) Part of this internship will be adding unit test coverage.
@lita: I'll take care of creating unit test tickets that split up the work between you and Ingrid.
Ingrid Cheung (added to the nosy list) is working on unit test scaffolding, inspired by the tkinter tests.
I want to make sure there's a clear course of action for Lita on this ticket. If cleanup is controversial, how about rescoping this to points (3) and (4) from the original ticket statement:
@terry, what do you think about that?
@lita, your PEP-8 and linter work has not been in vain. :) It'll come in handy local to where you fix bugs and add features down the road.
terryjreedy commentedon May 31, 2014
I like the proposal and would like to see it happen. My concern is to avoid having interns write patches that get rejected for non-technical reasons. I won't make any specific suggestions until I get more information either from the pydev thread
http://thread.gmane.org/gmane.comp.python.devel/147857
or from Gregor. I already emailed him directly, asking him to sign a contributor agreement and settle the matter of turtle maintenance.
Lita, please post a summary of the types of issues you have found (at most, say, 20). Some things are no-brainers, like adding missing spaces, as in 'a=3' to 'a = 3' or 'f (a,b = 3)' to 'f(a, b=3)', which also removes extra spaces. Rietveld's within-lines diffs make these easy to check. Other fixes are riskier.
rhettinger commentedon Jun 1, 2014
I would like to provide a final review before of any proposed changes.
Also, along the way, I would happy to provide suggestions for more substantive changes (instead of shallow PEP-8 or PyLint changes).
The primary defect in the modules is that the code got "adultified" somewhere along the way and needs to migrate back to the sort of straight-forward code that kids can read and model their code after.
I've use this code to help train adults to teach kids and found that it needs to use a simpler feature set.
terryjreedy commentedon Jun 1, 2014
I think the *some* of the 'adultification' that you refer to is a result of Gregor reimplementing turtle in a tkinter-independent intermediate language or two, the lowest layer of which he then implemented in tkinter as one particular backend. He intended to implement that next-to-lowest layer with other backends for use outside the stdlib. But unless we were to replace tkinter, that flexibility is just added and unneeded complexity for turtle in the stdlib.
I originally read turtle.py to learn how to program the tkinter canvas. I hoped to see how visible turtle actions, especially animations, translated to canvas calls. I just downloaded 2.5.6 turtle.py (26kb instead of 145kb). At first glance, it seems more suited to that particular need. You would find it a better example of 'straight-forward code' for your classes. I just ran it from 2.7.6 Idle and the end-of-file demo runs.
I have no knowledge of why the 2.5 turtle was replaced instead of being patched and upgraded. At the time, I was just a casual Python user who had never used turtle.
LitaCho commentedon Jun 2, 2014
Thank you so much for your support, Terry. I really appreciate all your feedback. Your comments have been very helpful as I am learning Turtle and Tkinter.
Most of my changes are related to spacing, visual indentation, consistent line spaces between method definitions, and making sure all the code falls within 80 lines. They have been all been minor changes that wouldn't change the logic of the code. I haven't gotten to removing the commented out code yet.
I will wait to commit this till I fix a bug in Turtle.
bitdancer commentedon Jun 2, 2014
A side note about tests: once a couple or three years ago I made it so that you could run 'make doctest' against the turtle documentation and it would work (displaying stuff on the screen). There's no checking of the output, but it proved the examples at least executed cleanly (with the help of some sphinx-hidden extra code).
For whatever it is worth, they still appear to run to completion:
Document: library/turtle
------------------------
1 items passed all tests:
313 tests in default
313 tests in 1 items.
313 passed and 0 failed.
Test passed.
terryjreedy commentedon Jul 23, 2014
In private email, Gregor indicated that he would like to be involved in turtle maintenance, and will be available about Aug. 1.
arhadthedev commentedon May 5, 2023
Current status:
In fact, we should do this for the whole library, no special treatment is required for
Lib/turtle.pyhere.Only three occurrences are left:
cpython/Lib/turtle.py
Line 105 in e5b8b19
cpython/Lib/turtle.py
Lines 143 to 144 in e5b8b19
cpython/Lib/turtle.py
Lines 601 to 602 in e5b8b19
Only two occurrences are left:
cpython/Lib/turtle.py
Lines 3754 to 3755 in e5b8b19
cpython/Lib/turtle.py
Lines 3756 to 3758 in e5b8b19
However, should we address these cosmetic issues?
terryjreedy commentedon May 5, 2023
Reformatting correct, comprehensible, and needed code is against PEP 8 unless part of a bigger review and refactoring. Code is needed if is removal, as opposed to replacement, breaks some intended function. Removing unneeded code, whether discovered by a linter or otherwise, is routine as it inhibits performance or comprehension. So such removals are not purely cosmetic.
I think all 3 commented-out chunks should be removed. I will prepare a PR with explanations. I will consider the XXX comments as I do so.
terryjreedy commentedon May 5, 2023
Except for these 5 items, I want to close this as I consider anything else as either done, obsolete, or something for another issue.
pythongh-65772: Clean-up turtle module
gh-65772: Clean-up turtle module (#104218)
pythongh-65772: Clean-up turtle module (python#104218)