Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable overrides in Model #66

Open
barneydobson opened this issue Feb 23, 2024 · 0 comments
Open

Enable overrides in Model #66

barneydobson opened this issue Feb 23, 2024 · 0 comments
Assignees
Labels
enhancement New feature or request overrides

Comments

@barneydobson
Copy link
Collaborator

# Initialise nodes
# Initialise arcs
# And then...
model.add_overrides(data.get("overrides", {}))

# And in Model
def add_overrides(self, config: Dict):
    for node in config.get("nodes", {}):
        type_ = node.pop("type_")
        name = node.pop("name")
        self.nodes_type[type_][name].apply_overrides(node)

    for arc in config.get("arcs", {}):
        name = arc.pop("name")
        self.arcs[name].apply_overrides(arc)

The node.apply_overrides and arc.apply_overrides will be the methods that will call the appropriate setters to ensure that any side effects of modifying those parameters are taken care of.

apply_overrides should be a method (not an abstract one) of the base class for nodes and arcs that will do nothing unless it is overridden. This way, only those specific nodes that can be overridden will need to have this method (and the relevant setters) implemented.

class Node:
    def apply_overrides(self, config: Dict):
        pass

class Arc:
    def apply_overrides(self, config: Dict):
        pass

In terms of how this can be done from a practical point of view, I would open issues for:

  1. Implement the above overriden logic. Should be harmless and backwards compatible with any existing code and config files.
  2. Identify which nodes/arcs have parameters that are worth to be overriden and open an issue for each of those nodes/arcs, indicating which parameters need such override. Watch out for recursive calls! You might need some hidden internal attributes that can be set directly and only use the setters in the overrides:
class MyNode:
    def __init__(...):
        # Does not use the setter
        self._some_param = get_default_some_param(...)

    @property 
    def some_param(self):
        return self._some_param

    @some_param.setter 
    def some_param(self, value):
        if not value:
            return
        # Some complex logic
        self._some_param = ...
        # More logic here

    def apply_overrides(self, config: Dict):
        self.some_param = config.pop("some_param", None)
        ...

Originally posted by @dalonsoa in #54 (comment)

@barneydobson barneydobson added enhancement New feature or request overrides labels Feb 23, 2024
@barneydobson barneydobson self-assigned this Feb 23, 2024
This was referenced Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request overrides
Projects
None yet
Development

No branches or pull requests

1 participant