[visionegg] Re: Obscure bug in visionegg
- From: Andrew Straw <astraw@xxxxxxxxxxx>
- To: visionegg@xxxxxxxxxxxxx
- Date: Sat, 16 Aug 2008 14:29:35 +0200
Dav Clark wrote:
Hi Andrew and the rest...
I've come across a strange bug in the ParameterTypes.py file in
VisionEgg (1.1). The following code illustrates a crash:
import VisionEgg
from VisionEgg.Text import Text
s = VisionEgg.Core.get_default_screen()
t = Text()
t.set(text='test')
If you look at the logic of ParameterTypes.get_type (line 326), you
see that before testing if 'test' is a string, it's tested to see if
it's a 4-item sequence, and then the string ends up as a Sequence4x4.
A bit confusing, but it happens because substrings (like 'test'[0])
are also sequences, and so pass the relatively weak test on line 331
of ParameterTypes.py. It would be a lot easier to simply throw the
thing at numpy.array and see if something sensible comes back.
So, in the code snippet above, t.set obtains that 'test' is a
Sequence4x4 (which it's not!). Then, on line 387 of
ParameterTypes.py, we catch an exception from assert_type each time
through, which leads to control flowing eventually to line 397, where
an undefined reference is made to require_class. So your program
crashes with the rather nonintuitive UnboundLocalError. This would
happen whenever an AnyOf type is not satisfied...
I guess you should raise a TypeError after the for loop ends (i.e.
after line 391 of ParameterTypes.py, dedented two stops, similar to
line 410)?
I could provide a patch, but the fix for assert_type is trivial, and
otherwise I'm not exactly sure what the right thing to do is here...
For example, I'm not sure any code expects to see these Sequence4x4
types. The only reference I can find to the type is in a comment on
line 1054 of Core.PerspectiveProjection - the type doesn't actually
seem to be used anywhere. More generally, it seems like there is
type-related stuff left over that is now completely unused - probably
a shift to using more standardized (python and numpy) types and shape
attributes would be in order... I understand why the code developed
it's own types, since types in python weren't great until at least
python 2.3, which came out a year after VisionEgg. But now it seems
the only useful one is "AnyOf".
Thanks for the report, Dav. I have fixed the 2 issues. For the 4
character string issue, I solved it by explicitly checking for string
instances before checking for sequence instances. The UnboundLocal error
was fixed along the lines you suggest. See
http://visionegg.org/trac/changeset/1470
As you note, type-checking in VE pre-dates the decent types
infrastructure introduced in Python (2.3?), but the reason for its
existence is more fundamental. The basic idea is to protect programmers
(actually, scientists without a lot of programming experience) from
shooting themselves in the foot by explicitly raising errors when
attempting to set variables to things that weren't going to work and
would raise more obscure errors down the track. (As with many cases in
Python when duck typing fails, PyOpenGL, to pick one example, doesn't
always give clear errors when the input arguments are not what it's
expecting.) If I were to redo the type-checking in any substantial way
(a thought which fills me with dread, although not without merit), I
would attempt to use enthought.traits, which also implements strict type
checking. I would also drop the entire __slots__ usage in VE, which is
an even worse example of defensive programming. These entire aspects of
the VE comes from a time when I was torn in many directions about what
it should be. A GUI application incorporating anything any experimenter
would want to do? A single API incorporating the same? A programming
library only for visual stimuli? As time passes, a programming library
for visual stimuli is the niche I see being sustainable for VE. The
high-level OpenGL wrapper is really the place where I think the VE can
be the right tool for the job and the thing it does well. With that in
mind, removing some or all of the defensive programming parts of VE
would actually make sense, particularly since those are some of the
trickier bits of code, as you have no doubt discovered. At least
switching to enthought.traits would help greatly in that regard,
although any major changes in that direction would be akin to open-heart
surgery on the software. (And in the domain of extensive changes, I
regard the idea of shifting to pyglet from PyOpenGL as higher priority
due to easy multi-display possibilities.)
I note that Sequence4x4 is used for all instances of ProjectionBaseClass
and its subclasses -- thus it is actually a fairly well used VE type.
(See line 763 of Core.py.)
I'm also happy to submit a bug to your Trac, but there don't seem to
be any real active tickets there... the only ticket being one that (I
think) you already accomplished!
Hmm, it appears I can't even log into Trac to close that bug -- so I
deleted it from the admin interface! I was having a lot of trouble with
spam on the wiki and trac, so I've locked things down more than I'd
like. With Trac it appears I locked them quite severely. Hmm, this will
take some time to sort out, and I'd rather migrate VE hosting to
something better anyway, so I'm just going to table this for now...
For now, the fix is simply not to use Parameter 'safe' code with
string or "AnyOf" types, because it's actually quite dangerous!
OK, please let me know if this change works for you. (Is checking out
from SVN OK, or should I make a tarball or zip file?)
This is a platform independent issue, but just for your reference, I'm
running on OSX 10.5.4. with the system python (2.5.1). (Why am I
running on the system python, you may ask? It's so I can use PyObjC 2
to control the bTop2 USB A/D box I've got).
Let me know how I can help!
Dav
ps - I've written a wrapper around VisionEgg which I'd like to share -
will anyone be at SciPy 2008? Might be a nice way to get some initial
feedback.
Unfortunately, I'll be away from Caltech during SciPy '08, but I wish I
could be there to talk with you...
======================================
The Vision Egg mailing list
Archives: //www.freelists.org/archives/visionegg
Website: http://www.visionegg.org/mailinglist.html
Other related posts:
