frequenz-floss / frequenz-sdk-python

Frequenz Python Software Development Kit (SDK)
https://frequenz-floss.github.io/frequenz-sdk-python/
MIT License
13 stars 17 forks source link

Fix Component Graph conversion from dict to class #789

Open daniel-zullo-frequenz opened 10 months ago

daniel-zullo-frequenz commented 10 months ago

What happened?

The Component Graph implementation is breaking the types. The Component Graph converts the input Components to dict to put the data in to the graph using the external graph library networkx which supports dicts. Then when calling methods to get the Components back, the Component Graph does not properly convert the dict to the strong type Component

Code to reproduce the bug:

import frequenz.sdk.microgrid.component_graph as gr
from frequenz.sdk.microgrid.client import Connection
from frequenz.sdk.microgrid.component import (
    Component,
    ComponentCategory,
    GridMetadata,
)
from frequenz.sdk.microgrid.fuse import Fuse
from frequenz.sdk.timeseries import Current

fuse = Fuse(max_current=Current.from_amperes(123.0))
components = {
    Component(1, ComponentCategory.GRID, None, GridMetadata(fuse)),
    Component(2, ComponentCategory.METER),
}
connections = {
    Connection(1, 2),
}

# pylint: disable=protected-access
graph = gr._MicrogridComponentGraph(components=components, connections=connections)

components = graph.components()
assert len(components) == 2
for component in components:
    if component.component_id == 1:
        assert component.category == ComponentCategory.GRID
        assert component.metadata is not None
        assert isinstance(component.metadata, dict)  # PASSED
        assert isinstance(component.metadata, GridMetadata)  # FAILED

What did you expect instead?

assert isinstance(component.metadata, dict)  # FAILED
assert isinstance(component.metadata, GridMetadata)  # PASSED

Affected version(s)

No response

Affected part(s)

Microgrid (API, component graph, etc.) (part:microgrid)

Extra information

The conversion from dict to Components is done using Component(**node) which does not work well when there are nested dicts.

The library marshmallow could be useful to deal with this issue as it does support nested classes from nested dicts.

daniel-zullo-frequenz commented 10 months ago

Note that this bug might be in all methods that convert from nested dict to class