open-cogsci / OpenSesame

Graphical experiment builder for the social sciences
http://osdoc.cogsci.nl/
GNU General Public License v3.0
241 stars 113 forks source link

Draw complex shapes with canvas.polygon (PsychoPy) #455

Closed eort closed 8 years ago

eort commented 8 years ago

Hey,

Currently, drawing complex (filled) shapes (concave,convex, holes, etc.) with the psychopy backend, is not possible. The issue is related to this issue on psychopy's github. By now, a newer version of psychpy (1.84) is out, which has fixed it.

Is there a reason why OpenSesame is still using the rather old version of psychopy (1.79)?

Eduard

smathot commented 8 years ago

Thanks! Yes, that could be implemented, possibly with a check whether the version of PsychoPy is recent enough.

Is there a reason why OpenSesame is still using the rather old version of psychopy (1.79)?

It isn't! That's probably the version that is shipped with your Ubuntu version. The Windows release of 3.1 uses PsychoPy 1.82.01.

eort commented 8 years ago

Ah, cool. I thought OpenSesame would have some dependencies that would break by using a newer PsychoPy version. Once, I updated psychopy, the issue disappeared.

Yes, that could be implemented, possibly with a check whether the version of PsychoPy is recent enough.

That's probably not necessary, as the new visual.ShapeStim just replaced the old one, implementing the fix. So from a certain version (1.81?) all shapes are drawn correctly. Maybe a warning would make sense if the version of PsychoPy is not recent enough.

smathot commented 8 years ago

Ok. You want to have a go at it?

eort commented 8 years ago

Hi Sebastiaan,

Here is a first draft. It is still quite ugly, but before I'd start the polishing work, I'd wanted to know whether this is the way these kinda things should be done. Baiscally, I added a version check of PsychoPy to the function ShapeStim function (located in Openexp\canvas\psycho.py). As PsychoPy is not imported earlier, I can't access its version attribute. Therefore, I load it, retrieve the version number, and delete it again afterwards. Obviously, this is bad, as this is done every time when ShapeStim is called. Also, I only print the warning and don't raise a proper warning, as OpenSesame would stop in latter case.

What do you think about it?

    def shapestim(self, vertices, fix_coor=True, close=False):

        """
        desc:
            Draws a stimulus definied by a list of vertices

            __Note:__

            Specific to the PsychoPy backend, primarily intended for internal
            use. Using this function directly will break your experiment when
            switching backends.

        arguments:
            vertices:   A list of lists, like [[0,0],[10,10]] containing the
                        vertices of the shape

        keywords:
            fix_coor:   A boolean indicating whether the vertices are in
                        OpenSesame or PsychoPy format.
            close:      Indicates whether the shape should be closed.
        """
        try:
            import psychopy
            psychpy_version = psychopy.__version__
            vers = [int(i) for i in psychpy_version.split('.')]
            if vers[0] == 1:
                if vers[1] < 84:
                    print(u'Your PsychoPy version is %s. Versions older than 1.84.0 \
are not able to draw complex shapes (e.g. convex shapes, holes, etc) \
To be able to draw them, you need to update your PsychoPy version'\
                         %psychpy_version)
                else:
                    print(u'Your PsychoPy version is %s. Versions older than 1.84.0 \
are not able to draw complex shapes (e.g. convex shapes, holes, etc) \
To be able to draw them, you need to update your PsychoPy version'\
                         %psychpy_version)
            elif vers[0]<1:
                print(u'Your PsychoPy version is %s. Versions older than 1.84.0 \
are not able to draw complex shapes (e.g. convex shapes, holes, etc) \
To be able to draw them, you need to update your PsychoPy version'\
                         %psychpy_version)  
        except: 
            pass
        try:
            del PsychoPy
        except:
            pass

        if fix_coor:
            _vertices = [self.to_xy(tuple(xy)) for xy in vertices]
        else:
            _vertices = vertices
        if self.fill:
            fill_color = self.color.backend_color
        else:
            fill_color = None
        stim = visual.ShapeStim(self.experiment.window, units=u"pix",
            lineWidth=self.penwidth, vertices=_vertices,
            lineColor=self.color.backend_color, closeShape=close,
            fillColor=fill_color, interpolate=False)
        self.stim_list.append(stim)

Eduard

smathot commented 8 years ago

Thanks!

Wait, I realize now that I misunderstood the whole problem! I thought you meant that drawing polygons was not possible, that it raised a NotImplementedError or something like that. But you mean that it is possible, but just gives funky results if the version of PsychoPy is too old. So we need to check and give user warning if this happens. Is that right?

Assuming that this is indeed the problem, then I have a few suggestions (mostly to do with general coding style):

Does that make sense?

eort commented 8 years ago

Hi Sebastiaan,

Thanks for the feedback. It was very helpful. Here, an updated version of ShapeStim (only showing the relevant part):

import psychopy, warnings
warnings.filterwarnings(action = 'once')
try:
    PsyPy_vers = StrictVersion(psychopy.__version__)
except ValueError:
    raise
warning_text = u'Your PsychoPy version is %s. Versions older than 1.84.0 \
are not able to draw complex shapes (e.g. convex shapes, holes, etc) \
To be able to draw them, you need to update your PsychoPy version' %PsyPy_vers
if PsyPy_vers.version[0] == 1:
    if PsyPy_vers.version[1] < 84:
        warnings.warn(warning_text,UserWarning)
    elif PsyPy_vers.version[0]<1:
        warnings.warn(warning_text,UserWarning)

The warning should be printed out only once, at the top of the script. Right now the user would see an enormous amount of output in the debug window with almost every drawing operation!

I left the warning in the function itself, but specified that it should be raised only once. Is this what you meant, or do you want the warning be raised really at the very top of psycho.py and being raised regardless of whether shapestim is called or not?

Thanks again, Eduard

smathot commented 8 years ago

or do you want the warning be raised really at the very top of psycho.py and being raised regardless of whether shapestim is called or not?

Yes, that's what I meant! shapestim is called pretty much always, because it's also used by rect and line.

And this:

warnings.filterwarnings(action = 'once')

Would affect all warnings, so it's not something that should be done just for this particular case.

Finally, a raise by itself is not valid Python, and the current structure doesn't deal with all three cases (i.e. up-to-date: do nothing, outdated: warn, or unknown: warn). A better way would be something like (so only warnings, but no crashes!):

try:
    psypy_version = StrictVersion(psychopy.__version__)
except ValueError:
    # Warn that version is unknown
    pass
else:
    if psypy_version.version < (1, 84):
        # Warn that version is too old
        pass

Do you see the logic? The else clause of a try .. except is executed only when no Exception occurs.