Neater syntax for thing settings exposed as properties

[rwb27]

A pattern has emerged in the OpenFlexure Microscope codebase, where some Things have many properties that look like:

@thing_property
def mysetting(self) -> str:
    """Documentation for the mysetting property"""
    return self.thing_settings.get("mysetting", "mydefaultvalue")

@mysetting.setter:
def mysetting(self, val: str):
    self.thing_settings["mysetting"] = val

It would clearly be neater to create a shorthand for this. A ThingSetting descriptor could subclass PropertyDescriptor and implement a property that changes the thing's settings directly.

There are a few options for how this might look in a Thing definition. I think we need a few things in whatever syntax we come up with:

• The name of the setting
• A type
• A default value
• A docstring
• A neat way to determine whether it should be readable or writeable over HTTP would be good too.

Ideas for discussion

Here are a few options which I've summarised from the discussion below:

• A decorated default function would define a function that returns the default value, and a decorator to turn it into a property that can be read and written. I quite like this syntax. My only worry is that it might confuse someone who is not aware that the function is called only once, to obtain the default value. After the first run, there will be a value in the settings file, so the default is not used.

  @thing_setting
  def mysetting(self) -> bool:
      """A setting to switch something on and off"""
      return False  # this would be the default value

• A property-like decorator would work similarly to the current solution, but hide the boilerplate related to saving it persistently. This is very much like how one might define a simple property using @property.:

  _my_setting: bool = False
  
  @thing_setting
  def mysetting(self) -> bool:
      """A setting to switch something on and off"""
      return self._my_setting
  
  @mysetting.setter:
  def mysetting(self, val: bool):
      self._my_setting = val

• An explicit descriptor looks less like a structure someone might already be using, but is very similar to what is done in many typing/serialisation libraries:

  mysetting = ThingSetting(default=False, type=bool, doc="A setting to switch something on and off")

• Using a type hint and field object along the lines of dataclasses.field or pydantic.Field (pydantic's docs are helpful):

  mysetting: bool = ThingSetting(default=False)
  """A setting to switch something on and off"""

  I particularly like that standard Python is used for the type and docstring, which should help autocompletion/introspection/type checking/autodoc tools.
• An annotated type hint looks perhaps most obviously like a simple property to me:

  mysetting: Annotated(bool, ThingSetting) = False
  """A setting to switch something on and off"""

There may be cases to consider requiring more options, in particular:

• Some settings might not want to be writeable over HTTP
• Some settings might not want to be readable over HTTP
• It may be desirable to allow Things to add settings not defined at the start - I'm less sure about this.

Discussion is welcome - I am keen to find syntax that makes sense to more people than just me!

[julianstirling]

My issue with:

@thing_property
def mysetting(self):
    return self.thing_settings["mysetting"]

@mysetting.setter(self, val):
def mysetting(self):
    self.thing_settings["mysetting"] = val

Is it doesn't look like a language feature, it feels very manual and open to mistakes in implementation. It is also quite a lot of changes to make a property a setting or vice-versa.

I am also not really a fan of:

@thing_setting
def mysetting(self) -> bool:
    """A setting to switch something on and off"""
    return False  # this would be the default value

it seems too automagical and different from normal python.

I would propose that

@thing_setting
def mysetting(self):
    return self._my_setting

@mysetting.setter
def mysetting(self, val):
    self._my_setting = val

has the benefit that. If I want to change a setting to a property I can just change @thing_setting to @thing_property and no other code changes. If I wanted it to be a normal python property I change @thing_setting to @property. It follows a standard pattern and no on who is used to writing python properties needs to look it up

The second iteration of changes on #110, I think answers the problem of changing the __class__ descriptor on the fly and saving each individual setting. Instead the class Thing has a way to know which of its attributes are settings. This way

my_thing.settings returns a list of the names of the settings in the above case ["mysetting"]. When saving the Thing can run {name: getattr(self, name) for name in self.settings} to create a dictionary of settings to save.

To the points above:

Some settings might not want to be writeable over HTTP

This seems like the normal way to do that is not to create a setter and to have a non HTTP exposed method that updates the underlying value? The setter does the saving. So probably we need a version of the decorator that sets the read only bool in the PropertyDescription or subclass thereof. So that the setter can be called from within the function but is not exposed to the client.

Some settings might not want to be readable over HTTP

If this is needed an @secret_thing_setting might need to be created?

It may be desirable to allow Things to add settings not defined at the start - I'm less sure about this.

I really don't like this, adding settings on the fly seems like it could cause chaos. If a thing specifically wants to be able to save arbitrary information it can always make a setting that is a dictionary, and then it is free to allow arbitrary key value pairs into that. This way only arbitrary data can be saved to a Thing if the Thing explicitly creates a setting for that arbitrary data.

[rwb27]

I agree having a decorated function that returns a default feels a bit strange. That said, it's not without precedent: it's exactly what the functools.cached_property decorator does (admittedly, cached properties can't be written to).

I think I did consider this form for regular properties too - it works whenever a property needs to be "dumb" (i.e. it works just like a variable that always takes the value you supply when you set it, and that's the value you get back when you get it). I didn't do it in the end, because I felt something called "property" ought to be a getter. I'm not sure that's such a problem if it's a "setting"?

I'm not convinced by the version with an explicit getter and setter, because while it looks more like a normal property, it's actually not - which creates potential for confusion. It would also need to handle _mysetting being missing, supply a default value, etc. - I think it becomes quite a bit of boilerplate which is exactly what we're trying to avoid, and I'm not sure it's any safer than the current syntax in terms of implementing it incorrectly.

The example you give also raises the question of what would happen if the setter and getter were something less trivial - I'm actually not clear how that should or would work. At present, properties-that-are-really-settings show explicitly how the values passed in and out of the descriptor map to the setting in the file. With a setting decorator, how would the example below work?

@thing_setting
def mysetting(self) -> str:
    return self._my_setting.upper()

@mysetting.setter:
def mysetting(self, val: str):
    if len(val) >99:
        val = val[:97] + "..."
    self._my_setting = val.lower()

That is perhaps slightly silly, but we probably do need to think through how it would work, if it's something we allow. A less silly example might be a setting that needs to be synchronised with the hardware. That's probably a question to kick into the future - but it suggests to me that we might want to restrict settings to dumb variables for now.

If the decorated default function gets thoroughly shouted down, a possible resolution would be to use explicit descriptor syntax, e.g.

class Whatever(Thing):
    mysetting = Setting(default=42, type=int, doc="some descriptive text")

That's done by lots of typing/serialisation libraries for this kind of thing, and I think this syntax already works for thing properties (though perhaps it could live with being more visible). With a bit of attention, the type hints should work OK as well.

A further option would be to use type hints as done by, for example, pydantic:

class Whatever(Thing):
    mysetting: Annotated(int, SettingAnnotation) = 42

This has the advantage of immediately looking like a "dumb" property (i.e. one that behaves very much like a variable) and having obvious places for type, default, and docstring. It could be turned into a descriptor using a metaclass, so the implementation would be the same.

Having written these down, I am slightly favouring the last option (annotated type).

[rwb27]

On the point about settings not defined at the start, I think I agree with you that it would be preferable to force explicit definition of everything that might go in there. That's not how it works at present, so it would be a (more) breaking change that probably does need some Things rewriting to make it work.

One case we should consider is the "camera controls" on StreamingPicamera2, which currently get put into a dictionary. I guess the resolution there is to create a setting called "camera_controls" to hold the dictionary as you suggest, so it's a relatively easy fix.

[rwb27]

I have summarised the various syntaxes we have suggested in the issue description - having done so, I think I am keen on the dataclasses-style syntax. It would, I think, work very much like your current implementation, but has the advantage of not needing any boilerplate code. There's also a clear place to specify whether it's read-only etc. as further arguments to ThingSetting. The only thing I'd want to check is how it works with typing, as it's not totally clear to me that ThingSetting would know it is supposed to be a bool except by looking at the default value. Clearly there's a way this works in dataclasses - it might require a metaclass I think.

[julianstirling]

The issue with the other syntaxes is that we have different syntaxes for Thing Property or Thing Setting, or Property. Which means changing them is a lot of work. The boilerplate is a bit over the top, but it is standard to basic python which means it is instantly recognisable.

Annotated I get is a built in feature, but I 100% do not understand it, and there are a PEPs specifically about trying to standardise it because it is confusing. So I don't think it makes it a simple way to do things.

Also if we do want to make a setting less dumb, we would need two different way to define the setting. One for dumb settings and one for "clever" ones. Quoting the zen of Python:

There should be one-- and preferably only one --obvious way to do it.

As for the comment:

I'm not convinced by the version with an explicit getter and setter, because while it looks more like a normal property, it's actually not - which creates potential for confusion. It would also need to handle _mysetting being missing, supply a default value, etc. - I think it becomes quite a bit of boilerplate which is exactly what we're trying to avoid, and I'm not sure it's any safer than the current syntax in terms of implementing it incorrectly.

You just define the default value at __init__ or at when defining that the variable exists. Linters will enforce that this is done. This is the standard way that all properties hold default values.

I would like to follow standard programming patterns of Python otherwise we need to train everyone using this framework to learn new patterns. Which however well documented, will be confusing and difficult.

For the syntax:

class Whatever(Thing):
    mysetting = Setting(default=42, type=int, doc="some descriptive text")

If we change "SettingDescriptor" to "Setting" this is possible based on the implementation in #110, using the @thing_setting decorator.

[julianstirling]

Confirmed that using SettingDescriptor works in the #110 implementation. While I don't love having two ways to do things, it does give you the option to make a setting read only by setting the readonly argument. And gets rid of the need to create a setter for a readonly setting.

I would say that what a descriptor is is very very under the hood in python and is not a concept we can expect all people using the framework to understand. I would recommend changing:

• PropertyDescriptor to ThingProperty
• SettingDescriptor to ThingSetting

So you can either do:

_my_setting = 25.0
@thing_setting
def mysetting(self) -> float:
    """A setting, and it belongs to me"""
    return self._my_setting

@mysetting.setter
def mysetting(self,  val:float):
    self._my_setting = val

OR

ThingSetting(initial_value=25.0, model=float, description="A setting, and it belongs to me")

Note that the arguments are the ones currently defined, they could be updated. I am not a fan of model, I think type is more standard Python language, but it does shadow a built-in.

[rwb27]

I agree the top-level API probably shouldn't have "descriptor" in the name. If settings are to be created using SettingDescriptor I agree calling it ThingSetting makes much more sense. I have not found myself making much use of PropertyDescriptor directly, but renaming it (and ActionDescriptor) in a similar way might be a good idea. To be honest, there are plenty things that could do with renaming, but obviously it's annoying when names change. It might be one to fix when we simplify the imports.

PropertyDescriptor is possibly guilty of being a class that tries to do too much. It not only provides a work-a-like for Python's built in property, it also provides functionality that implements the "standard" behaviour, and adds a few hooks for Events in the bargain. That we've basically only ever used it via the decorator is a function of my lack of documentation, and the fact that I ended up mostly using that syntax because it worked best for what I was doing with it.

[rwb27]

Re: the Zen of Python - there are already two ways to define an attribute that functions like a variable:

class MyClass:
    def __init__(self):
        self._myothersetting = 42

    mysetting: int = 42
    """Here is a docstring"""

    _myothersetting: int
    @property
    def myothersetting(self) -> int:
        return self._myothersetting

    @myothersetting.setter
    def myothersetting(self, val: int) -> None:
        self._myothersetting = val   

The descriptor protocol means users of that class can use the same syntax for either attribute. There are very few reasons to use the second form, unless you're adding functionality, i.e. either the getter or setter is not the trivial example above.

It is important to remember the Zen of Python is an aspiration rather than a rule, and that even the standard library does not live up to it everywhere (see the example above). The dataclass-style option in the issue description is what's done by both pydantic and dataclasses, so I think it can be argued it's quite standard already. It makes both the type hint and the docstring more obvious and more standard, avoids Annotated, plays nicely with mypy, and avoids boilerplate code that has the potential to introduce errors.

I think it's OK to have a simple form and a more complicated form. This is done in many places in both the Python language and its popular frameworks.

I might prototype some code to see how it looks, but I think it is worth getting some opinions from others: it seems clear we have quite different takes on this one.

[JohemianKnapsody]

Anything with setter and getter separately, I'd vote to avoid unless we can't

class Autofocus(Thing):
    autofocus_dz = ThingSettings(initial_value=2000, model=int, description="The distance, in steps, to autofocus over")

and then in a client or anywhere else we need that setting, using either self.autofocus_dz or Autofocus.autofocus_dz looks good to me?

model instead of type is slightly odd to me, but I'd get over it fairly quickly.

I think having all our settings in one block at the start is nice as well. The difference between

autofocus_dz = ThingSettings(initial_value=2000, model=int, description="The distance, in steps, to autofocus over")
autofocus_method = ThingSettings(initial_value="fast", model=str, description="The autofocus method to use")

and the space given to each having multi-line setters and getters seems like a big win to me?

[VigneshVSV]

Hi guys, not fully following, but please have a look at my implementation for this. Configuration management through properties is a solved problem (not by me, but since many years by others).

I have three flags on properties called 'db_init' , 'db_commit' and 'db_persist' to solve this.

The management is carried out by the property descriptor only.

db_init - load property from a database or file at the initialization of the Thing. setter must be called only after connection with the deivce.

db_commit - stores the property onto a database or file post set operation

db_persist - is a combination of init and commit

You would most likely need a post_init method to implement this correctly. Moreover, you may want to different between db_init and db_post_init which are properties are written before init and post init respectively.

This is something I carried over from a framework called tango controls.

[julianstirling]

OK, so coming together. We have agreed that using a descriptor if not named "Descriptor" is a way to minimise boilerplate for creating a property.

I think still in terms of their is one way of doing things. I think we should follow Python's syntax for making a "clever" property, using the decorator/getter/setter only for extra functionality.

[julianstirling]

it seems clear we have quite different takes on this one.

I don't understand this statement, the implementation I wrote already has the way to do it with descriptors. I just want a different name for them. I was in favour of not recommending two syntaxes, but you and Joe have convinced me that for something dumb, it is probably cleaner.

Also it solves the issue that you can set readonly for a read only settings. I have tested this for the microscope and it reliably saves.

[rwb27]

I think perhaps I was misunderstanding - my bad. If we are happy with the simpler syntax for dumb properties/settings, I think we have a consensus. We should go with the first descriptor option as that's what's implemented, and I think the dataclass style syntax can be a future MR (which can be backwards compatible).

In order to keep that option open, it would be ideal to have a function called ThingSetting, that returns a SettingDescriptor, rather than renaming the class (this is in line with how both dataclass and pydantic do it, and it makes typing work better).