classification
Title: missing update() in _Screen.setup() of Lib/turtle.py
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, georg.brandl, gpolo, gregorlingl, loewis
Priority: normal Keywords: patch

Created on 2008-10-13 23:02 by gregorlingl, last changed 2010-10-21 20:35 by belopolsky. This issue is now closed.

Files
File name Uploaded Description Edit
_Screen.setup.diff gregorlingl, 2008-10-13 23:02 bugfix
setup_bug_demo.py gregorlingl, 2008-11-20 19:57 demonstrates the __init__ - update bug
setup_patch.diff gregorlingl, 2008-11-20 19:59
setup_bug_demo.py gregorlingl, 2008-11-20 20:14
Messages (14)
msg74710 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-10-13 23:02
In Lib/turtle.py

_Screen.setup() needs a final update() call in order
to ensure, that immediately following calls of window_width() or
window_height() return correct values.

Consequently the setup() method call in _Screen.__init__()
has to take place only after the call of TurtleScreen.__init__()
msg74721 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-10-14 04:52
This patch applies to Python 2.6 as well
Gregor
msg76022 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-18 22:19
I'd also like to see this patch accepted and done for Python 2.6.1 and
(at the same time) python 3.0 before the last rc is released. So perhaps
you could you dedicate a few minutes to this one also.

Thanks for your efforts,
Gregor
msg76029 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-11-19 00:05
Why are you using update instead of update_idletasks ? 
I'm not talking exactly about this line being added, but this ends up
calling TurtleScreenBase._update which calls self.cv.update(), cv being
a canvas. update_idletasks should be used exactly for performing the
kind of tasks expected here, display update and window layout
calculations, while calling update may also process other events or
pending errors.
msg76046 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-19 13:31
I cannot call the Canvas method _update_idletasks() from within
_Screen.setup() becaus this would contradict to the architecture of the
module which isolates all direct references to Tkinter to
TurtleScreenBase. (The idea behind this is to make the module easily
portable, by porting only this class).

So if one followed your proposition one had to change TurtleScreenBase
which would result in a different (additional) patch.

For now I'd like to stick with the one proposed here - I worked a lot
with it, it works and I didn't experience unwanted side effects.

My question: I suppose to change the call self.cv.update() to
self.cv.update_idletasks() in TurtleScreenBase._update wouldn't work
properly so one had to add a new method to TurtleScreenBase - something
like TurtleScreenBase._update_idletasks. Originally I had the intention
to keep this interface as small as possible and use only things that are
really needed. Did you experience any problems or undesired behaviour
because of using unly cv.update? If so, please report it. Perhaps you
could also provide an example.

Regards,
Gregor
msg76071 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-19 21:04
> I cannot call the Canvas method _update_idletasks() from within
> _Screen.setup() becaus this would contradict to the architecture of the
> module which isolates all direct references to Tkinter to
> TurtleScreenBase. (The idea behind this is to make the module easily
> portable, by porting only this class).

I find that desire misguided; this is (IMO) a case of false abstraction.
Is there any kind of proof that this design actually
works, i.e. can be ported to a different GUI library (like, say,
PythonWin? or AWT, when run in Jython?)

Unless such proof can be provided (and then integrated into the code),
I recommend to give up that objective, and start merging the base class
code into the subclasses where reasonable.

> Did you experience any problems or undesired behaviour
> because of using unly cv.update?

I agree with with gpolo: it's a general Tk programming principle to
defer updates whenever possible. This allows the event handler to return
more quickly, making the system more responsive. Each individual update
call will only contribute a small amount of time to the response time.
It's many of these which eventually make the entire system sluggish.
msg76077 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-19 23:11
> > I find that desire misguided; this is (IMO) a case of false abstraction.
> > Is there any kind of proof that this design actually
> > works, i.e. can be ported to a different GUI library (like, say,
> > PythonWin? or AWT, when run in Jython?)
   
Yes there is. I have a (nearly complete) port to Pygame. (The only 
exception is the ondrag method, which I will incorporate if I have time
for it.) It runs all of the turtle graphics example scripts (except
colormixer which uses ondrag) without problems.

> > Unless such proof can be provided (and then integrated into the code),
> > I recommend to give up that objective, and start merging the base class
> > code into the subclasses where reasonable.

Sorry, but I definitely shall not follow your recommendation.  I have
presented the architecture of the turtle module at Europython 2006  in a
talk which was visited also by Guido v. Rossum and later in Leipzig at a
workshop where you yourself, Martin, was present. On both of these
occasions I showed working prototypes of this port (along with another
one to VPython) and nobody had any objections nor were there any
objections by other useres who have used it up to now against this
design. You can find this also in the "Tagungsband" to the Leipzig
Python workshop together with some screenshots.

I'm very confident that this is a good design and I know (form the 
experience mentioned above) that it works. So instead I'll proceed with 
porting it to Jython and I for my part would consider it as an advantage
to have the same turtle module in both of these flavours of Python.

Two more remarks on this discussion, a specific one and a general one.

1. The bug I submitted a patch for is in the method setup() and in the 
__init__ method of _Screen, both of which usually will be called only 
once in a program.So in *this* case it cannot cause any performance 
problems. The bug  has annoying consequences and I found a simple 
remedy, which I consider appropriate for a bug-fix-release like 2.6.1. I
don't see any reason why to keep a known bug like this one in the code.
Acceptance of the patch will certainly not affect any more fundamental
amendments to follow (possibly). Moreover up to now I didn't hear of a
single complaint about the performance of the turtle module. I think
this is (i) because for beginning programmers this is no issue and (ii)
there are means (to call tracer() and also to call update() directly) to
control performance to a considerable extent. Shortly, my opinion is
that there are *good reasons* why the implementation is done like this,
seen from the educator's point of view.

2. Is it really fruitful to discuss  general design issues along with
(comparatively) small problems like this one - in the sense of
alternative ways to fix that problem? Implementations of redesigns like
the one you recommended wouldn't be accepted anyway for bug fix releases 
or releases in rc phase. I'm open to fundamental criticism also, but I 
think that should be led in Python-Dev or possibly comp.lang.python. And 
working out an amended concept would take some time and also it's 
implementation and it's testing. Moreover, to find co-workers to work on 
this would be an advantage.

Regards,
Gregor
msg76081 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-19 23:41
> 2. Is it really fruitful to discuss  general design issues along with
> (comparatively) small problems like this one - in the sense of
> alternative ways to fix that problem?

Most definitely. The module went into Python without any review
whatsoever. Nobody (but you) has ever looked at the code in detail.
Only now that I look at it I wonder whether the design of the code
is appropriate.

In this specific case, gpolo suggested a reasonable change to the
proposed patch. You opposed this change, pointing out that this
change contradicts the design behind it. As I think the change gpolo
requested is desirable, it *must* be the design that is wrong (as
it restricts us from doing what I think should be done).

So: if you bring up "the design" as a reason for doing things the
way they are done, expect the design to be challenged.

You might argue that with due process, review should have taken
place before the code was integrated. You might be right, but then
the new turtle module wouldn't have been part of Python 2.6.

Wrt. the specific design issue: I believe that attempts to provide
cross-platform GUI in a simple fashion are doomed to fail. Java
AWT is an extraordinary example of it, but many more libraries
exist that essentially prove that cross-platform GUI is a bad idea;
one may argue that Tk itself is also proof of that (although
it is fairly sophisticated, and not at all simple). The fact that
I have not objected to it earlier is simply because I must have
ignored any claims about cross-platform GUIs.
msg76087 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-20 02:13
> Most definitely. The module went into Python without any review
> whatsoever. Nobody (but you) has ever looked at the code in detail.

That's not True! Brad Miller, for example, who also had submitted
patches to the pythontracker, coauthor of "Python Programming in
Context",  has used a predecessor of turtle.py as a main tool (swiss
army knife, as he says) in his book. He has contributed a few patches
(via private communication, before the module went into the Python
trunk), one of them directly concerning the update method. He had also
suggested some of the features, which I have added towards the end of
the development.

> You might argue that with due process, review should have taken
> place before the code was integrated. You might be right, but then
> the new turtle module wouldn't have been part of Python 2.6.

Rigth, more or less. At least I had expected, that someone reads the
doc-strings of the approx. 15 classes in the module. The one of
TurtleScreenBase reads like this:

"""Provide the basic graphics functionality.
Interface between Tkinter and turtle.py.

To port turtle.py to some different graphics toolkit
a corresponding TurtleScreenBase class has to be implemented.
"""

Gregor
msg76131 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-20 19:57
The attached file setup_bug_demo.py shows the bug of this issue. It's
taken out of a working game application and radically abridged to
concentrate on this issue.

Now the patch to fix the bug has to be changed a bit (because of the
changes of Martin's Singleton fix). A diff file follows immediately
(setup_patch.diff).

I tried out (in a simple straight forward way and in contradiction with
my ideas mentioned in a previous posting) 

(1) to replace the update() call by _Screen._canvas.update_idletasks() and
(2) to do a similar replacement in TurtleScreenBase._update()

Neither of these work. The second change even affects seriously the
working of the modules demo().

I'd like to mention that in the former turtle module upto 2.5 (remember:
the compatibility to it was a requirement for the new one) there are
also several Canvas.update() calls but no Canvas.update_idletasks() call.

I never noticed negative consequences of this and as far as I know,
neither concerning the old module nor the new one there were ever
complaints concerning bad ("sluggish") performance due to this (or any
other) fact.
msg76132 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-20 19:59
Here the patch, updated
msg76135 - (view) Author: Gregor Lingl (gregorlingl) Date: 2008-11-20 20:14
Sorry, but there is a remain from testing different versions of the
turtle module in the demo file's import statement.

Should read (of course):

from turtle import Screen, Turtle, mainloop

A corrected version is attached

Sorry, again, for the inconvenience
Gregor
msg119150 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-10-19 17:59
It looks like this has been fixed in turtle 1.1.  See r72318 and issue 5923.
msg119335 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-10-21 20:35
See also "turtle.py update for 3.1" post on python-dev specifically mentioning this bug as fixed.

http://mail.python.org/pipermail/python-dev/2009-May/089383.html
History
Date User Action Args
2010-10-21 20:35:03belopolskysetstatus: pending -> closed

messages: + msg119335
2010-10-19 17:59:32belopolskysetstatus: open -> pending

nosy: + georg.brandl, belopolsky
messages: + msg119150

resolution: out of date
stage: resolved
2008-11-20 20:14:08gregorlinglsetfiles: + setup_bug_demo.py
messages: + msg76135
2008-11-20 19:59:03gregorlinglsetfiles: + setup_patch.diff
messages: + msg76132
2008-11-20 19:57:55gregorlinglsetfiles: + setup_bug_demo.py
messages: + msg76131
2008-11-20 02:13:51gregorlinglsetmessages: + msg76087
2008-11-19 23:41:10loewissetmessages: + msg76081
2008-11-19 23:11:29gregorlinglsetmessages: + msg76077
2008-11-19 21:04:41loewissetmessages: + msg76071
2008-11-19 13:31:47gregorlinglsetmessages: + msg76046
2008-11-19 00:05:04gpolosetnosy: + gpolo
messages: + msg76029
2008-11-18 22:19:27gregorlinglsetmessages: + msg76022
2008-10-14 04:52:44gregorlinglsetmessages: + msg74721
versions: + Python 2.6
2008-10-13 23:02:43gregorlinglcreate