param icon indicating copy to clipboard operation
param copied to clipboard

Keep `names` attribute and `objects` attribute of an `ObjectSelector` in sync

Open sdc50 opened this issue 6 years ago • 2 comments

Now that the ObjectSelector accepts a mapping as the objects argument of the constructor it would be nice to also allow the objects attribute of the class to be set with a mapping. Otherwise, if the objects attribute is changed after instantiation then the names attribute is out of sync. For example:

image

One suggestion would be to make objects a property that has a setter:

class ObjectSelector(Selector):
...
     __slots__ = ['_objects','compute_default_fn','check_on_set','names']

    @property
    def objects(self):
        return self._objects

    @objects.setter
    def objects(self, objects):
        if objects is None:
            objects = []
        if isinstance(objects, collections.Mapping):
            self.names = objects
            self._objects = list(objects.values())
        else:
            self.names = None
            self._objects = objects

    # ObjectSelector is usually used to allow selection from a list of
    # existing objects, therefore instantiate is False by default.
    def __init__(self,default=None,objects=None,instantiate=False,
                 compute_default_fn=None,check_on_set=None,allow_None=None,**params):
        self.objects = objects
        ...

Then the behavior would be like I would expect: image

sdc50 avatar Dec 14 '18 19:12 sdc50

This should be fixed before we release 1.9.

philippjfr avatar Dec 14 '18 19:12 philippjfr

I'm not sure what the behavior should be when check_on_set is False and the property is set to a value that is not in objects. Perhaps the _ensure_value_is_in_objects method should update the names attribute. Maybe something like this would work:

    def _ensure_value_is_in_objects(self,val):
        """
        Make sure that the provided value is present on the objects list.
        Subclasses can override if they support multiple items on a list,
        to check each item instead.
        """

        if not (val in self.objects):
            self.objects.append(val)
            if self.names is not None:
                self.names[hashable(val)] = val

sdc50 avatar Dec 14 '18 19:12 sdc50