Facades as dataclasses

[jnnr]

This is a draft to show how facades can be implemented as dataclasses. The advantages are:

• explicit definition of class attributes
• type annotation
• Clearer handling of required and default arguments

Special attention has been given to make sure that the behaviour of the classes regarding required and default arguments does not change. This has been checked manually, via the constraint tests and by re-running and comparing an energysystem in the model oemof-B3.

Note: ElectricalLine is imported from oemof.solph but does not inherit from the base class Facade. It has not been changed into a dataclass. It should move to the experimental package once that has been set up.

Problems encountered:

• the facades original __init__ passes args and kwargs to the parent class (with super().__init__(*args, **kwargs), which makes it possible to set attributes of the base class that are not explicit in the facade (like invest_relation_output_capacity for storage). To keep this behaviour, I implemented a wrapper that adds that call to the __init__.
• Declaring the facade as dataclass may override the __hash__ method that is defined in oemof.network.Node (the hash of the class is defined as the hash of the label). Applying the correct settings (unsafe_hash=False, frozen=False, eq=False, see 6fb68f5), the __hash__ method remains untouched.

[henhuy]

Hi @jnnr, like your approach to use pythons dataclass very much!
Only thing which bothers me, is your second decorator to use kwargs in dataclasses.
Could you explain where this is needed? Could not find a case where this is needed.
As I understand a new class Storage could implement missing attribute invest_relation_output_capacity itself, or not?

[jnnr]

Thanks for your feedback, @henhuy!

As I understand a new class Storage could implement missing attribute invest_relation_output_capacity itself, or not?

Good point - in this way, all attributes would have to be defined explicitly in the facade, which is good.
Arguments like invest_relation_output_capacity, which are used optionally, would be None by default.
Will have to test if this has other implications. I will try this with Dispatchable and Storage. If it works, we can adapt all other facades.

Update: This has more implications:

• "_facade_requires_" would not be passed to parent class Facade. This is ok because dataclasses can handle required/optional attributes
• Setting self.type and adding subnodes happens in Facades' init, too. The decorator could still prepend the super() call to the dataclasses init, but without passing kwargs. Would that look ok to you, @henhuy?

[henhuy]

Also saw, that *args and **kwargs in Facade are also forwarded to Nodes __init__ function.
Thus, maybe some other kwargs may not be covered in case of fixed attr?
Argh - those kwargs are really annoying.
So maybe we stay with your approach.

[jnnr]

Thanks for continuing this, @monika-o!
You can have a look at results of the constraint tests (in github actions or locally at your computer by running pytest tests/test_constraints.py to find out where your changes change the lp files (they represent the optimization problem). These tests compare the lp-files that are generated for each component to a predefined default. Goal is that they remain the same.

[jnnr]

TODOs:

Test

• Test with oemof-B3 model instances: preprocessed Datapackages and results are identical.

[jnnr]

I checked for all facade classes that have been adapted if their behavior has changed and found only few lines to correct.
In addition, the _facade_requires seem not to be used any more and could be dropped in the class Facade.

[jnnr]

@henhuy, @MaGering. @srhbrnds : This feature is almost finished and should help working with tabular in the future. Please have a look or a review or just be informed. The main improvement is that by making the facades into dataclasses we can transparently show their attributes, datatypes and defaults.

@henhuy: I adapted the decorator kwargs_to_parent, if you have the time please have a look or let us take a look together.
@MaGering: This should not alter anything in your model. If you can, it might be good to have another check, i.e. to run your models with tabular on this branch.