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

Mypy error in certain formula engine #951

Closed matthias-wende-frequenz closed 3 weeks ago

matthias-wende-frequenz commented 1 month ago

What happened?

When adding a Power object to a formula engine mypy complains with different error messages

Example 1

On checking this following code:

consumer_excess_power_engine = (
    microgrid.producer().power
    + Power.from_watts(-1000)
).build("excess_power")

mypy reports:

error: Incompatible types in assignment (expression has type "Any | None", variable has type "Sample[Any]")  [assignment]
Found 1 error in 1 file (checked 1 source file)

Example 2

On checking this following code:

consumer_excess_power_engine = (
    microgrid.producer().power
    + microgrid.consumer().power
    + Power.from_watts(-1000)
).build("excess_power")

mypy reports:

error: "Sample3Phase[Any]" has no attribute "value"; maybe "value_p1", "value_p2", or "value_p3"?  [attr-defined]
Found 1 error in 1 file (checked 1 source file)

Example 3

On contrast checking this following, having the Power object in the middle code works:

consumer_excess_power_engine = (
    microgrid.producer().power
    + Power.from_watts(-1000)
    + microgrid.consumer().power
).build("excess_power")

Example 4

This code doesn't work.

consumer_excess_power_engine = (
    Power.from_watts(-1000)
    + microgrid.producer().power
).build("excess_power")
find_bug.py:21: error: "Power" has no attribute "build"  [attr-defined]
find_bug.py:23: error: Unsupported operand types for + ("Power" and "FormulaEngine[Power]")  [operator]
Found 2 errors in 1 file (checked 1 source file)

What did you expect instead?

For the examples 1-3 that work mypy shouldn't complain. We may investigate a way to make Example 4 work.

Affected version(s)

v1.x.x

Affected part(s)

Data pipeline (part:data-pipeline)

Extra information

No response

shsms commented 1 month ago

I'm unable to reproduce, and it is hard for me to believe these errors are from these lines, because they don't make sense for the above lines, even if there is a bug. It is likely that these came from somewhere else. If you have a reproducible example, please share.

matthias-wende-frequenz commented 1 month ago

This is the example app that I was using

import asyncio

from datetime import timedelta
from frequenz.sdk import microgrid
from frequenz.sdk.actor import ResamplerConfig
from frequenz.sdk.timeseries import Power

async def run() -> None:
    # This points to the default Frequenz microgrid sandbox
    microgrid_host = "microgrid.sandbox.api.frequenz.io"
    microgrid_port = 62060

    # Initialize the microgrid
    await microgrid.initialize(
        microgrid_host,
        microgrid_port,
        ResamplerConfig(resampling_period=timedelta(seconds=1)),
    )

    # negative means feed-in power due to the negative sign convention
    consumer_excess_power_engine = (
        microgrid.producer().power
        + microgrid.consumer().power
        + Power.from_watts(-1000)
    ).build("excess_power") # (2)!
    cons_excess_power_recv = consumer_excess_power_engine.new_receiver()

    async for cons_excess_power in cons_excess_power_recv:
        cons_excess_power = cons_excess_power.value
        if cons_excess_power is None:
            continue
        print(f"Consumer excess power: {cons_excess_power}")

def main() -> None:
    asyncio.run(run())

if __name__ == "__main__":
    main()

and you where right, the bug is not in the shared lines but when accessing the received value.

find_bug.py:28: error: "Sample3Phase[Any]" has no attribute "value"; maybe "value_p1", "value_p2", or "value_p3"?  [attr-defined]
Found 1 error in 1 file (checked 1 source file)
daniel-zullo-frequenz commented 1 month ago

@matthias-wende-frequenz ~that error might be due to the channels latest upgrade where value has been renamed to message~

See release notes here

Selected

The value property was renamed to message.

edited: nevermind, the code is not using select

shsms commented 1 month ago

Ok, that makes sense. The overrides all seem to have been made for the 3-phase formulas rather than the 1-phase formulas, for which these operations on single constant values are available: https://github.com/frequenz-floss/frequenz-sdk-python/blob/ca89486bdad5bddff960c0b86a4425bb530b6c0a/src/frequenz/sdk/timeseries/formula_engine/_formula_engine.py#L848-L851