traits icon indicating copy to clipboard operation
traits copied to clipboard

Simplify Expression trait

Open ievacerny opened this issue 4 years ago • 2 comments

At the moment Expression trait makes use of setattr_original_value and has a validation method that modifies given value. This has caused issues with default value setting (https://github.com/enthought/traits/issues/1087, https://github.com/enthought/traits/issues/1096).

The use case for Expression seems pretty simple - original trait (e.g. expression) holds the "source" which is used for inspection and the shadow/mapped trait (e.g. expression_) holds the compiled code object. 2 current uses of Expression are in TraitsUI table_column.py and table_filter.py which fall under this use case.

Given this simple use of Expression it could avoid issues with setattr_original_value by being simplified as follows:

class Expression(TraitType):
    default_value = "0"
    info_text = "a valid Python expression"

    is_mapped = True

    def validate(self, object, name, value):
        try:
            compile(value, "<string>", "eval")
            return value
        except:
            self.error(object, name, value)

    def post_setattr(self, object, name, value):
        object.__dict__[name + "_"] = self.mapped_value(value)

    def mapped_value(self, value):
        return compile(value, "<string>", "eval")

Note: removing the use of setattr_original_value flag from Expression means that tests added in https://github.com/enthought/traits/issues/1087 no longer cover the targeted code branch in ctraits.c.

ievacerny avatar May 19 '20 15:05 ievacerny

Sounds good to me. Putting in the 6.2.0 milestone (but I won't complain if we find time to do it for 6.1.0).

mdickinson avatar May 20 '20 09:05 mdickinson

Bumping.

mdickinson avatar Oct 04 '21 13:10 mdickinson