classification
Title: Clean up turtle.py code formatting
Type: enhancement Stage: needs patch
Components: Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: jesstess Nosy List: Lita.Cho, anish.shah, ezio.melotti, gregorlingl, ingrid, jesstess, r.david.murray, rhettinger, terry.reedy
Priority: normal Keywords: gsoc

Created on 2014-05-25 01:48 by jesstess, last changed 2016-02-06 09:04 by anish.shah.

Messages (12)
msg219068 - (view) Author: Jessica McKellar (jesstess) * (Python triager) Date: 2014-05-25 01:48
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:

1. Run turtle.py through a pep8 checker and fix the issues that are reasonable to fix.

2. Run turtle.py through a linter like pyflakes and fix reasonable linter issues.

3. Examine commented out code, and either remove it or open a ticket if it represents an issue that should be fixed.

4. 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.
msg219072 - (view) Author: Lita Cho (Lita.Cho) * Date: 2014-05-25 05:14
I'm claiming this ticket. Plan to work on it during my OPW internship.
msg219410 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-05-30 18:46
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.
msg219427 - (view) Author: Lita Cho (Lita.Cho) * Date: 2014-05-30 20:46
Hi Terry,

> is there project link? are any of the mentors core developers, with
>  commit rights? or would you need commits from someone like me?

I am not 100% sure. Let me ask Jessica, who is my mentor, and get back to you.

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

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.

> 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 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 pep8 compliant and work through linter. I am also trying to delete some of the commented out code.

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

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.
msg219438 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-05-31 04:00
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.
msg219462 - (view) Author: Jessica McKellar (jesstess) * (Python triager) Date: 2014-05-31 16:58
Terry, thank you for all the time you've been putting into the GSoC and OPW tickets.

> Questions: is there project link? are any of the mentors core developers, with commit rights? or would you need commits from someone like me?

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.

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

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.

> 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).

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:

> 3. Examine commented out code, and either remove it or open a ticket if it represents an issue that should be fixed.

> 4. 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.

@Terry, what do you think about that?

@Lita, your pep8 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.
msg219476 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-05-31 21:42
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.
msg219524 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-06-01 20:34
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.
msg219536 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-01 23:58
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.
msg219537 - (view) Author: Lita Cho (Lita.Cho) * Date: 2014-06-02 00:30
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.
msg219610 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-06-02 17:41
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.
msg223765 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-07-23 19:18
In private email, Gregor indicated that he would like to be involved in turtle maintenance, and will be available about Aug. 1.
History
Date User Action Args
2016-02-06 09:04:38anish.shahsetnosy: + anish.shah
2014-07-23 19:18:02terry.reedysetmessages: + msg223765
2014-06-02 17:41:16r.david.murraysetnosy: + r.david.murray
messages: + msg219610
2014-06-02 00:31:00Lita.Chosetmessages: + msg219537
2014-06-01 23:58:06terry.reedysetmessages: + msg219536
2014-06-01 20:34:10rhettingersetnosy: + rhettinger
messages: + msg219524
2014-05-31 21:42:41terry.reedysetmessages: + msg219476
2014-05-31 16:58:51jesstesssetnosy: + ingrid
messages: + msg219462
2014-05-31 16:08:39ezio.melottisetnosy: + ezio.melotti
2014-05-31 04:00:56terry.reedysetmessages: + msg219438
2014-05-30 20:46:51Lita.Chosetmessages: + msg219427
2014-05-30 18:46:21terry.reedysetnosy: + terry.reedy
messages: + msg219410
2014-05-25 05:14:18Lita.Chosetmessages: + msg219072
2014-05-25 03:51:05ned.deilysetnosy: + gregorlingl
2014-05-25 02:44:34Lita.Chosetnosy: + Lita.Cho
2014-05-25 01:48:24jesstesscreate