python-microscope / microscope

Python library for control of microscope devices, supporting hardware triggers and distribution of devices over the network for performance and flexibility.
https://www.python-microscope.org
GNU General Public License v3.0
70 stars 41 forks source link

setting an enum Setting sets to the value instead of an enum #70

Closed carandraug closed 5 years ago

carandraug commented 5 years ago

The new Setting class and device settings systems handles enum types. If a device setting is an enum, the settings system will have the client return the enum value instead of the enum instance. However, the setter for a setting does not do this.

Consider this device:

class Mode(enum.Enum):
    A = 1
    B = 2

class Thing(microscope.device.Device):
    def __init__(self):
        super().__init__()
        self._mode = Mode.A # type: Mode
        self.add_setting('mode', 'enum', self._get_mode, self._set_mode, Mode)

    def _get_mode(self) -> Mode:
        print("returning '%s' of class '%s'"
              % (self._mode, type(self._mode)))
        return self._mode

    def _set_mode(self, new_mode: Mode) -> None:
        print("setting to '%s' of class '%s'"
              % (new_mode, type(new_mode)))
        self._mode = new_mode

In this case, _get_mode() returns an instance of <enum 'Mode'> and the paired _set_mode() takes an instance of <enum 'Mode'> too.

The settings system recognizes that the returned value is an enum and returns the enum value instead.

>>> x = Thing()
>>> val = x.get_setting('mode')
>>> print("received '%s' of class '%s'" % (val, type(val)))
returning 'Mode.A' of class '<enum 'Mode'>'
received '1' of class '<class 'int'>'

However, when setting the value, no reverse conversion is done:

>>> x.set_setting('mode', 2)
setting to '2' of class '<class 'int'>'

So far, this has only been used in the new Andor camera class and all the enums in there are IntEnum which are also ints where this should not be an issue.

carandraug commented 5 years ago

Here's a suggested fix https://github.com/carandraug/microscope/commit/c637f237f8cf492d6b81c52c92762ec4cc755849

It also removes the conversion of setted value from enum instance to its value. If the Setting getter does not return an enum instance, the setter should not accept one.

carandraug commented 5 years ago

Fixed and tests adjusted with a67bbd9 . Closing.