kalekundert / byoc

MIT License
0 stars 0 forks source link

Add `on_load()` callbacks to Config/param objects #4

Closed kalekundert closed 3 years ago

kalekundert commented 3 years ago

Right now, parameters that depend on other parameters are a big weakness for appcli. For example, consider an object with attributes a, b, and c, where c is derived from a and b via f(a, b) and f is expensive. For a normal object, it'd make sense to calculate c each time a or b was set. Although this snippet is somewhat long, it's simple and robust:

@autoprop
class Obj:

    def get_a(self):
        return self._a

    def set_a(self, a):
        self._a = a
        self._calc_c()

    def get_b(self):
        return self._b

    def set_b(self, a):
        self._b = b
        self._calc_c()

    def get_c(self):
        return self._c

    def _calc_c(self):
        self._c = f(self._a, self._b)

This pattern isn't compatible with appcli, though. The reason is that appcli parameters are basically equivalent to the getters above, and there are no equivalents to the setters. In other words, appcli doesn't provide a way to call setters like those in the above examples. That forces the user to avoid setters and do everything with getters. This means either (i) recalculate c every time or (ii) implement some sort of caching scheme:

# Simple but inefficient:
@autoprop
class Obj:
    a = appcli.param()
    b = appcli.param()

    def get_c(self):
        return f(self.a, self.b)
# Efficient but complex, especially if multiple caches are needed.
# Unit tests would be advised, because nasty bugs are definitely possible.
@autoprop
class Obj:
    a = appcli.param()
    b = appcli.param()

    def get_c(self):
        if self._cache != (self.a, self.b):
            self._c = f(self.a, self.b)
            self._cache = (self.a, self.b)

        return self._c

I think it'd be better if appcli had the equivalent of a setter. More specifically, this would be a callback invoked after parameters are set by init() or load(). This might look like:

@autoprop
class Obj:
    a = appcli.param()
    b = appcli.param()

    def __load__(self):
        self.c = f(self.a, self.b)

Some thoughts: