2

Similar questions has been asked before over the years. Python2 and Python3 seem to work the same. The code shown below works fine and is understandable (at least, to me). However, there is a no-op that bothers me and I wonder if there might be a more elegant way of expressing this functionality.

The key issue is that when a subclass sets a new value of a property in a variable defined in the superclass, the superclass should save the newly modified value to a file. The way the code below does this is for each subclass to have a line like this, which is a no-op, in a setter method:

self.base_data_property.fset(self, super().data) 

I call this a no-op because the superclass's data has already been modified by the time this line is executed, and the only reason for that line of code to exist is for the side effect of triggering the superclass's @data.setter method, which performs the automatic saving to a file.

I don't like writing side-effecty code like this. Is there a better way, other than the obvious, which is:

super().save_data()  # Called from each subclass setter

The above would called instead of the no-op.

Another critizism of the code below is that the super()._base_data is not obviously a superset of the subclass lambda_data. This makes the code hard to maintain. This results in the code seems to be somewhat magical because changing a property in lambda_data is actually aliases to changing a property in super()._base_data.

Code

I made a GitHub repo for this code.

import logging


class BaseConfig:

    def __init__(self, diktionary):
        self._base_data = diktionary
        logging.info(f"BaseConfig.__init__: set self.base_data = '{self._base_data}'")

    def save_data(self):
        logging.info(f"BaseConfig: Pretending to save self.base_data='{self._base_data}'")

    @property
    def data(self) -> dict:
        logging.info(f"BaseConfig: self.data getter returning = '{self._base_data}'")
        return self._base_data

    @data.setter
    def data(self, value):
        logging.info(f"BaseConfig: self.data setter, new value for self.base_data='{value}'")
        self._base_data = value
        self.save_data()


class LambdaConfig(BaseConfig):
    """ This example subclass is one of several imaginary subclasses, all with similar structures.
    Each subclass only works with data within a portion of super().data;
    for example, this subclass only looks at and modifies data within super().data['aws_lambda'].
    """

    def __init__(self, diktionary):
        super().__init__(diktionary)
        # See https://stackoverflow.com/a/10810545/553865:
        self.base_data_property = super(LambdaConfig, type(self)).data
        # This subclass only modifies data contained within self.lambda_data:
        self.lambda_data = super().data['aws_lambda']

    @property
    def lambda_data(self):
        return self.base_data_property.fget(self)['aws_lambda']

    @lambda_data.setter
    def lambda_data(self, new_value):
        super().data['aws_lambda'] = new_value
        self.base_data_property.fset(self, super().data)

    # Properties specific to this class follow

    @property
    def dir(self):
        result = self.data['dir']
        logging.info(f"LambdaConfig: Getting dir = '{result}'")
        return result

    @dir.setter
    def dir(self, new_value):
        logging.info(f"LambdaConfig: dir setter before setting to {new_value} is '{self.lambda_data['dir']}'")
        # Python's call by value means super().data is called, which modifies super().base_data:
        self.lambda_data['dir'] = new_value
        self.base_data_property.fset(self, super().data)  # This no-op merely triggers super().@data.setter
        logging.info(f"LambdaConfig.dir setter after set: self.lambda_data['dir'] = '{self.lambda_data['dir']}'")


    @property
    def name(self):  # Comments are as for the dir property
        return self.data['name']

    @name.setter
    def name(self, new_value):  # Comments are as for the dir property
        self.lambda_data['name'] = new_value
        self.base_data_property.fset(self, super().data)


    @property
    def id(self):  # Comments are as for the dir property
        return self.data['id']

    @id.setter
    def id(self, new_value):  # Comments are as for the dir property
        self.lambda_data['id'] = new_value
        self.base_data_property.fset(self, super().data)


if __name__ == "__main__":
    logging.basicConfig(
        format = '%(levelname)s %(message)s',
        level = logging.INFO
    )

    diktionary = {
        "aws_lambda": {
            "dir": "old_dir",
            "name": "old_name",
            "id": "old_id"
        },
        "more_keys": {
            "key1": "old_value1",
            "key2": "old_value2"
        }
    }

    logging.info("Superclass data can be changed from the subclass, new value appears everywhere:")
    logging.info("main: Creating a new LambdaConfig, which creates a new BaseConfig")
    lambda_config = LambdaConfig(diktionary)
    aws_lambda_data = lambda_config.data['aws_lambda']
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info("")

    lambda_config.dir = "new_dir"
    logging.info(f"main: after setting lambda_config.dir='new_dir', aws_lambda_data['dir'] = {aws_lambda_data['dir']}")
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info(f"main: aws_lambda_data['dir'] = '{aws_lambda_data['dir']}'")
    logging.info("")

    lambda_config.name = "new_name"
    logging.info(f"main: after setting lambda_config.name='new_name', aws_lambda_data['name'] = {aws_lambda_data['name']}")
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info(f"main: aws_lambda_data['name'] = '{aws_lambda_data['name']}'")

    lambda_config.id = "new_id"
    logging.info(f"main: after setting lambda_config.id='new_id', aws_lambda_data['id'] = {aws_lambda_data['id']}")
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info(f"main: aws_lambda_data['id'] = '{aws_lambda_data['id']}'")

Output

INFO Superclass data can be changed from the subclass, new value appears everywhere:
INFO main: Creating a new LambdaConfig, which creates a new BaseConfig
INFO BaseConfig.__init__: set self.base_data = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data setter, new value for self.base_data='{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: Pretending to save self.base_data='{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO main: aws_lambda_data = {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}
INFO 
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO LambdaConfig: dir setter before setting to new_dir is 'old_dir'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'old_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'new_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data setter, new value for self.base_data='{'aws_lambda': {'dir': 'new_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: Pretending to save self.base_data='{'aws_lambda': {'dir': 'new_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'new_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO LambdaConfig.dir setter after set: self.lambda_data['dir'] = 'new_dir'
INFO main: after setting lambda_config.dir='new_dir', aws_lambda_data['dir'] = new_dir
INFO main: aws_lambda_data = {'dir': 'new_dir', 'name': 'old_name', 'id': 'old_id'}
INFO main: aws_lambda_data['dir'] = 'new_dir'
INFO 
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'new_dir', 'name': 'old_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data setter, new value for self.base_data='{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: Pretending to save self.base_data='{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO main: after setting lambda_config.name='new_name', aws_lambda_data['name'] = new_name
INFO main: aws_lambda_data = {'dir': 'new_dir', 'name': 'new_name', 'id': 'old_id'}
INFO main: aws_lambda_data['name'] = 'new_name'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'old_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data getter returning = '{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'new_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: self.data setter, new value for self.base_data='{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'new_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO BaseConfig: Pretending to save self.base_data='{'aws_lambda': {'dir': 'new_dir', 'name': 'new_name', 'id': 'new_id'}, 'more_keys': {'key1': 'old_value1', 'key2': 'old_value2'}}'
INFO main: after setting lambda_config.id='new_id', aws_lambda_data['id'] = new_id
INFO main: aws_lambda_data = {'dir': 'new_dir', 'name': 'new_name', 'id': 'new_id'}
INFO main: aws_lambda_data['id'] = 'new_id'
Mike Slinn
  • 7,705
  • 5
  • 51
  • 85
  • I've yet to find a good way to subclass `property` (though I haven't tried terribly hard). One alternative is to define your own property-like descriptor to use in place of `property`. – chepner Oct 16 '20 at 13:18

2 Answers2

0

The fact that .fset is not triggered automatically has nothing to do with the property being defined in a superclass. If you set self.data on the subclass, the setter will be triggered seamlessly as expected.

The problem is that you are not setting self.data. The property refers to a mutable object, and you are making changes (setting new keys) in that object. All the property mechanism get in a line like super().data['aws_lambda'] = new_value is a read access to super().data - Python resolves that part of the expression and returns a dictionary - then you set the dictionary key.

(BTW, the super() call is redundant there, if you don't redefine data as a property in the subclass - and likely not doing what you'd want if such an overider property were set anyway - you could (and likely should) just be using self.data in these accesses).

Anyway, you are not the only person with this problem - the SQLAlchemy ORM also suffers from this and goes out of its way to provide ways to signal a dictionary (or other mutable value) in an instrumented attribute is "dirty", so that it is flushed to the database.

You are left with two options: (1) explicitly triggering the data save, which is what you are doing; (2) Use a specialized derived dictionary class that is aware it should trigger the saving when it is changed.

The approach in (2) is elegant, but takes a lot of work to be done correctly - you'd need at least a Mapping and a Sequence specialized classes implementing these patterns in order to support nested attributes. It will work reliably if done correctly, though.

Since you are encapsulating the dictionary values in class attributes already, (thus doing (1)), and it works seamlessly for your class' users, I'd say you could keep it that way. You might want an explicit, _ prefixed method to force saving the parameter in the super class, instead of manually triggering the property setter, but that is it.

Ah, yes, and like I said above:

    @lambda_data.setter
    def lambda_data(self, new_value):
        data = self.data
        data['aws_lambda'] = new_value
        # Trigger property setter, which performs the "save":
        self.data = data

No need for all those super() calls, and setattr.

if you feel confortable with Python "lens" (link in the comments), it can be used to write your setters in a single line - since it both sets the new value and returns the mutated object.

You can have problems with that in concurrent code - if one piece of code is holding and changing self.data, and it is replaced by a whole new object returned by the lens:

from lenses import lens

    @lambda_data.setter
    def lambda_data(self, new_value):
        self.data = lens['aws_lambda'].set(new_value)(self.data)

    ...
    @dir.setter
    def lambda_data(self, new_value):
        self.lambda_data = lens['dir'].set(new_value)(self.lambda_data)

(what makes this work is that we are actually calling the setters for each property, with the new object created by the lens call)

jsbueno
  • 99,910
  • 10
  • 151
  • 209
  • Thanks for your quick response. If I remove those `super()` calls I get `NameError: name 'data' is not defined`. Also, I wonder if a lens might be helpful (https://python-lenses.readthedocs.io/en/latest/tutorial/intro.html). – Mike Slinn Oct 16 '20 at 13:54
  • Your `lambda_data` method is a significant improvement. – Mike Slinn Oct 16 '20 at 14:02
  • How would you write `@dir.setter` in `LambdaConfig`? – Mike Slinn Oct 16 '20 at 14:07
  • It could be done just the same way lambda_data.setter is done. Also, in your 1st comment, I can't see how `self.data` would raise NameError - (just "data" is a local variable, it has to be defined first) – jsbueno Oct 16 '20 at 14:16
  • `lens` could work to reduce your setters to a single line. If you are confortable with the answer with an example – jsbueno Oct 16 '20 at 14:22
  • I got it to work as you suggested. Understanding the solution hurts my brain, but the pattern is simple to follow. A lens approach would might seem a bit easier to understand. I'll mark your solution as accepted. Thanks! – Mike Slinn Oct 16 '20 at 14:42
0

I redid my original code using the approach described by @jsbueno and optimized it:

import logging


class BaseConfig:

    def __init__(self, _dictionary):
        self._base_data = _dictionary
        logging.info(f"BaseConfig.__init__: set self.base_data = '{self._base_data}'")

    def save_data(self):
        logging.info(f"BaseConfig: Pretending to save self.base_data='{self._base_data}'")

    @property
    def data(self) -> dict:
        logging.info(f"BaseConfig: self.data getter returning = '{self._base_data}'")
        return self._base_data

    @data.setter
    def data(self, value):
        logging.info(f"BaseConfig: self.data setter, new value for self.base_data='{value}'")
        self._base_data = value
        self.save_data()


class LambdaConfig(BaseConfig):
    """ This example subclass is one of several imaginary subclasses, all with similar structures.
    Each subclass only works with data within a portion of super().data;
    for example, this subclass only looks at and modifies data within super().data['aws_lambda'].
    """

    # Start of boilerplate; each BaseConfig subclass needs something like the following:

    def __init__(self, _dictionary):
        super().__init__(_dictionary)

    @property
    def lambda_data(self):
        return self.data['aws_lambda']

    @lambda_data.setter
    def lambda_data(self, new_value):
        data = self.data
        data['aws_lambda'] = new_value
        self.data = data  # Trigger the super() data.setter, which saves to a file

    def generalized_setter(self, key, new_value):
        lambda_data = self.lambda_data
        lambda_data[key] = new_value
        # Python's call by value means the super().data setter is called, which modifies super().base_data:
        self.lambda_data = lambda_data

    # End of boilerplate. Properties specific to this class follow:

    @property
    def dir(self):
        return self.data['dir']

    @dir.setter
    def dir(self, new_value):
        self.generalized_setter("dir", new_value)


    @property
    def name(self):
        return self.data['name']

    @name.setter
    def name(self, new_value):
        self.generalized_setter("name", new_value)


    @property
    def id(self):
        return self.data['id']

    @id.setter
    def id(self, new_value):
        self.generalized_setter("id", new_value)


if __name__ == "__main__":
    logging.basicConfig(
        format = '%(levelname)s %(message)s',
        level = logging.INFO
    )

    diktionary = {
        "aws_lambda": {
            "dir": "old_dir",
            "name": "old_name",
            "id": "old_id"
        },
        "more_keys": {
            "key1": "old_value1",
            "key2": "old_value2"
        }
    }

    logging.info("Superclass data can be changed from the subclass, new value appears everywhere:")
    logging.info("main: Creating a new LambdaConfig, which creates a new BaseConfig")
    lambda_config = LambdaConfig(diktionary)
    aws_lambda_data = lambda_config.data['aws_lambda']
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info("")

    lambda_config.dir = "new_dir"
    logging.info(f"main: after setting lambda_config.dir='new_dir', aws_lambda_data['dir'] = {aws_lambda_data['dir']}")
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info(f"main: aws_lambda_data['dir'] = '{aws_lambda_data['dir']}'")
    logging.info("")

    lambda_config.name = "new_name"
    logging.info(f"main: after setting lambda_config.name='new_name', aws_lambda_data['name'] = {aws_lambda_data['name']}")
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info(f"main: aws_lambda_data['name'] = '{aws_lambda_data['name']}'")
    logging.info("")

    lambda_config.id = "new_id"
    logging.info(f"main: after setting lambda_config.id='new_id', aws_lambda_data['id'] = {aws_lambda_data['id']}")
    logging.info(f"main: aws_lambda_data = {aws_lambda_data}")
    logging.info(f"main: aws_lambda_data['id'] = '{aws_lambda_data['id']}'")
    logging.info("")

    logging.info(f"main: lambda_config.data = {lambda_config.data}")
Mike Slinn
  • 7,705
  • 5
  • 51
  • 85