Factory functions for device initialization#32
Conversation
065647e to
e0d38e3
Compare
`hrl.graphics.new_graphics(...)`creates the correct graphics object based on a specified alias. These aliases are defined in a map (`hrl.graphics.ALIAS_MAP`). Thus, adding a new device would only require adding it to this map and the factory function.
Prevents importing all the possible devices; only import the branch we need
Leaves all the switching logic there, instead of in the `HRL`-class
e0d38e3 to
57d4a8c
Compare
guillermoaguilar
left a comment
There was a problem hiding this comment.
small correction needed in the aliases mappings, and to add analogous for inputs and photometers
| alias for the desired graphics device, by default "gpu". | ||
| Valid options are defined in hrl.graphics.ALIAS_MAP. | ||
| inputs : str | ||
| alias for the desired input device, by default "keyboard". |
There was a problem hiding this comment.
shouldn't we here also give possible options, as in graphics?
"Valid options are defined in ..."
| inputs : str | ||
| alias for the desired input device, by default "keyboard". | ||
| photometer : str or None | ||
| alias for the desired photometer device, by default None. |
| mouse: enables or disables the mouse cursor. Default: False | ||
| graphics : str | ||
| alias for the desired graphics device, by default "gpu". | ||
| Valid options are defined in hrl.graphics.ALIAS_MAP. |
There was a problem hiding this comment.
isn't it GRAPHIC_ALIASES? line 33 of graphics/init.py
Added a commit for this now: all three hardware types have the same setup in terms of aliases being defined in the module. These can be accessed through |
|
Ok changes look good. so 'gpu' does not default to 'gpu_gray'. |
Added this alias now as well. |
Should this example in some way become part of the test suite or documentation? I've ignored it (the My take would be that we shouldn't have an If so, I'd probably make some changes as well -- mainly to use the Python standard library timing. Nowadays, that one is highly accurate across platforms. And, reading the vendorized PsychoPy clock, on Linux it already defaults to that anyway... |
Adds factory functions to construct and setup graphics devices, input devices and photometers.
The public-facing API of the
HRL-class does not change, making this change backwards compatible.Status quo, problem
Currently, the
HRL-class contains a lot of logic for selecting and setting up the correct device. This works through 'device aliases'; strings such as"gpu_grey", or"keyboard". The user specifies a device alias, and theHRL-class constructs the corresponding object.This is an undesirable tight coupling between
HRL-class and the devices. It requires a lot of this switching logic to be in theHRL-class. It also means that add more devices (or making changes to devices such as different parameters for initialization) requires updating of all this switching logic. In the end, it means that theHRL-object "needs to know" about the implementations of the graphics devices. This breaks the advantages of the Object-Oriented structure that we have. ( #11 )Solution
This pull request moves all of that device intialization and switching logic, to dedicated factory functions, e.g.,
hrl.graphics.new_graphics(...),hrl.inputs.new_inputs(..).HRL-object then calls these factory functions.This moves all of the logic out of the
HRL-class, decoupling it from the implementation classes. Thus, theHRL-class doesn't need to know which graphics object was created -- it can just use it.The one coupling that does remain in the
HRL-class is that it takes the.devicefrom the graphics object (which may hold the binding to a *Pixx device), asHRL.device, and then passes that tonew_input()where it can be used to initialize a DATAPixx device.So, HRL coordinates between graphics, inputs, and photometer, rather than manage the individual devices.
Implementation details
The submodule
graphics/__init__.py,inputs/__init__.py,photometer/__init__.pydefine the factory functions, so that they can be called ashrl.graphics.new_graphics(). The factory functions take the same device alias strings thatHRLpreviously did, and return an instantiated object of the correct subclass.Inside these factory functions, the switching logic is set up so that potentially multiple aliases can refer to the same class to be constructed, e.g.,
'gpu', 'gpu_grey', 'gpu_gray'all refer to the same device class.NOTE: one can still bypass the
new_graphics()(andHRL-class) entirely, and just construct a device specific object directly:The factory functions themselves also don't do any device specific initialization -- that's handled by the device-specific class. Thus, the
new_graphics()does not need 'to know' about setting up, e.g., a ViewPixx vs. DataPixx. Only one device-specific module gets imported at a time, which also makes this more robust.Future proofing
Adding a new device-class thus is simpler: write the device-class as normal (inhereting from the correct super-class, e.g.,
Graphics_greyorGraphics_GPU). Then, add it with one or more aliases tonew_graphics():... # Lazy import the graphics class based on alias if graphics_alias in ["gpu", "gpu_grey", "grey", "gray", "gray8"]: from .gpu import GPU_grey as graphics_class elif graphics_alias in ["new_device"]: from .new_device import DEVICE_grey as graphics_class ...Screen setup
Additionally, there is some necessary setting up of screens on multimonitor setups, via setting environment variables. This used to be done by the
HRL-object as well. Now, it is in ahrl.graphics.setup_screen()function. This function can be called separately, but is also automatically called bynew_graphics().