Problems with `defaults` and `constants`

[smathot]

The problem

Right now, configuration options are stored in the module defaults and overwritten by values from the module constants if available. This works fine when you indeed use a constants.py file to set options. But if you try to change options at runtime things get rough.

Basically, what happens is that all modules start like this:

from pygaze.defaults import *
try:
    from constants import *
except:
    pass

What this does is copy the values into the module itself. This means that changing an option in defaults, like so ....

from pygaze import defaults
defaults.DISPTYPE = 'opensesame'

... only works when no modules have been loaded yet. If a module has already been loaded, it needs to explicitly reloaded in order for the option to take effect. For the plug-ins, this required a hack, which is implemented in pygaze_init.reload_pygaze().

• https://github.com/esdalmaijer/PyGaze/blob/master/opensesame_plugins/pygaze_init/pygaze_init.py#L83

Another problem is that you can specify many options through keywords as well as the config system. When you actually start using the keywords, things easily get messy and confusing.

A solution

In general terms, there should be a config system in which all options are stored in a single place, without any copying of config options. (Copying now happens implicitly due to the import structure shown above.)

There are many ways to do this, but one way would be to mimick the config system of OpenSesame.

• https://github.com/smathot/OpenSesame/blob/master/libqtopensesame/misc/config.py

Basically, there is a single instance of the config class, called cfg (a singleton design pattern), and this can be accessed from anywhere in the code, like so:

from libqtopensesame.misc.config import cfg
cfg.my_option = 'a value' # Set the option
print cfg # Get the option

Perhaps the constants.py file could be read and then used to set options through this config system on start-up.

I would also do away with all the keyword arguments, such as disptype, that should really be set through the config system.

What do you think?

[esdalmaijer]

Basically, the problem is how to do this as easy as possible. The beauty of the constants module approach was really simple: just define everything that needs to remain constant in a single file, and then import that file. Of course I do see the problem that occurs with the keyword arguments. Bloody object oriented programmers redefining all sorts of crap at runtime... ;)

I've been thinking about a almost the exact approach as you propose for a while. Basically, the idea is to keep a dict (lets call it settings) that contains all of the values used in PyGaze's modules. It could be accessible via pygaze.settings, and when PyGaze is initialized it would contain all default values (possibly imported from defaults and constants, for backwards compatibility).

import pygaze

# print the default option, or the option set in constants.py, e.g. "pygame"
print(pygaze.settings['disptype'])

# set a new value
pygaze.settings['disptype'] = "psychopy"

I suppose that if settings is a global variable in the modules, this should work. Your approach in OpenSesame is more advanced, in that it's a class. I suppose that settings.disptype = "pygame" is a more sensible syntax than settings['disptype'] = "pygame", so I think I prefer your option.

[smathot]

Yes, a dict will work equally well. The benefit of using a class, and override the __setattr__() and __getattr()__ methods is, in addition to providing a clean syntax, that you can implement some extra functionality. For example you could raise informative Exceptions if settings are missing or load settings from constants.py on startup. But of course, all of these things are also possible using a dict, so it's a matter of taste.

I would suggest to put the settings object (be it a custom class or a dict) in a separate submodule though, because it's a conceptually distinct chunk of functionality. If you still want to be able to access it as pygaze.settings (which makes sense), you can use a redirect import in pygaze/__init__.py. For example like so:

# Redirect import
from pygaze.settings import settings

Btw, I removed all the other redirect imports from pygaze/__init__.py, so at the moment you cannot do from pygaze import Screen etc. anymore. This is not because I think this is a bad idea, but because these imports caused all modules to be loaded as soon as anything from pygaze was imported, thus causing the settings-copying issue described above. As soon as there is a better settings mechanism they can go back.

[esdalmaijer]

I noticed they went missing, and figured this was the reason. My preference goes out to using a class, due to the clearer syntax, that has the added bonus of resembling a Matlab struct (which means Matlab/PsychToolbox users will potentially adopt to PyGaze easier).

I agree on keeping the init as clean as possible; this seems like a thing for the misc module. This would still need to be one of the very first things to import in init, though! I'll see if I can whip up something this weekend; shouldn't be too difficult, but will require a lot testing to make sure everything runs smoothly, and backwards compatibility isn't compromised.

On Thu, Jan 9, 2014 at 9:32 AM, Sebastiaan Mathot notifications@github.comwrote:

Yes, a dict will work equally well. The benefit of using a class, and override the setattr() and getattr() methods is, in addition to providing a clean syntax, that you can implement some extra functionality. For example you could raise informative Exceptions if settings are missing or load settings from constants.py on startup. But of course, all of these things are also possible using a dict, so it's a matter of taste.

I would suggest to put the settings object (be it a custom class or a dict) in a separate submodule though, because it's a conceptually distinct chunk of functionality. If you still want to be able to access it as pygaze.settings (which makes sense), you can use a redirect import in pygaze/init.py. For example like so:

Redirect importfrom pygaze.settings import settings

Btw, I removed all the other redirect imports from pygaze/init.py, so at the moment you cannot do from pygaze import Screen etc. anymore. This is not because I think this is a bad idea, but because these imports caused all modules to be loaded as soon as anything from pygaze was imported, thus causing the settings-copying issue described above. As soon as there is a better settings mechanism they can go back.

— Reply to this email directly or view it on GitHubhttps://github.com/esdalmaijer/PyGaze/issues/13#issuecomment-31914897 .