Skip to content

Allow targeting categories with subtypes #168

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

Draft
wants to merge 2 commits into
base: v0.x.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ classifiers = [
requires-python = ">= 3.11, < 4"
dependencies = [
"typing-extensions >= 4.6.1, < 5",
"frequenz-api-dispatch == 1.0.0-rc1",
# "frequenz-api-dispatch == 1.0.0-rc1",
"frequenz-api-dispatch @ git+https://github.com/frequenz-floss/frequenz-api-dispatch@4274d1a",
"frequenz-client-base >= 0.8.0, < 0.10.0",
"frequenz-client-common >= 0.1.0, < 0.4.0",
"grpcio >= 1.66.1, < 2",
"grpcio >= 1.70.0, < 2",
"python-dateutil >= 2.8.2, < 3.0",
]
dynamic = ["version"]
Expand Down Expand Up @@ -93,7 +94,8 @@ dev-pylint = [
"pylint == 3.3.6",
# For checking the noxfile, docs/ script, and tests
"frequenz-client-dispatch[cli,dev-mkdocs,dev-noxfile,dev-pytest]",
"frequenz-api-dispatch == 1.0.0-rc1",
# "frequenz-api-dispatch == 1.0.0-rc1",
"frequenz-api-dispatch @ git+https://github.com/frequenz-floss/frequenz-api-dispatch@4274d1a",
]
dev-pytest = [
"pytest == 8.3.5",
Expand Down
3 changes: 2 additions & 1 deletion src/frequenz/client/dispatch/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ def print_dispatch(dispatch: Dispatch) -> None:
# Format the target
if dispatch.target:
if len(dispatch.target) == 1:
target_str: str = str(dispatch.target[0])
(el,) = dispatch.target
target_str: str = str(el)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is because target is a frozenset now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but why can't use use [0] for something frozen? You should be able, or is it because it is a set now?

Also what the hell does el stands for? It look it is pretty cryptic. Also if you need to clarify in the PR, maybe it is best to add a comment in the code itself for the future ourselves looking at this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot index-access a set, it has no order :)

el stands for element.. sure I'll make it more clear 😄
I was surprised as well, but this seems to be the easiest way to access the only element of a set with size 1

else:
target_str = ", ".join(str(s) for s in dispatch.target)
else:
Expand Down
73 changes: 67 additions & 6 deletions src/frequenz/client/dispatch/_cli_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,23 @@

import json
from datetime import datetime, timedelta, timezone
from sre_constants import IN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet this was Tab-oriented-programming + auto-import 😆

Suggested change
from sre_constants import IN

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was more desperation, but this is a draft after all :P

from typing import Any, Literal, cast

import asyncclick as click
import parsedatetime # type: ignore
from tzlocal import get_localzone

from frequenz.client.common.microgrid.components import ComponentCategory
from frequenz.client.dispatch.types import (
BatteryType,
CategoryAndType,
EvChargerType,
InverterType,
TargetCategories,
TargetCategoryAndTypes,
TargetIds,
)

# Disable a false positive from pylint
# pylint: disable=inconsistent-return-statements
Expand Down Expand Up @@ -140,7 +150,7 @@ class TargetComponentParamType(click.ParamType):

def convert(
self, value: Any, param: click.Parameter | None, ctx: click.Context | None
) -> list[ComponentCategory] | list[int]:
) -> TargetIds | TargetCategories | TargetCategoryAndTypes:
"""Convert the input value into a list of ComponentCategory or IDs.

Args:
Expand All @@ -149,9 +159,13 @@ def convert(
ctx: The Click context object.

Returns:
A list of component ids or component categories.
A list of targets, either as component IDs or component categories.
"""
if isinstance(value, list): # Already a list
if (
isinstance(value, TargetIds)
or isinstance(value, TargetCategories)
or isinstance(value, TargetCategoryAndTypes)
):
return value

values = value.split(",")
Expand All @@ -162,20 +176,67 @@ def convert(
error: Exception | None = None
# Attempt to parse component ids
try:
return [int(id) for id in values]
return TargetIds([int(id) for id in values])
except ValueError as e:
error = e

# Attempt to parse as component categories, trim whitespace
try:
return [ComponentCategory[cat.strip().upper()] for cat in values]
return TargetCategories(
[ComponentCategory[cat.strip().upper()] for cat in values]
)
except KeyError as e:
error = e

def valid_short_types() -> (
dict[str, BatteryType | InverterType | EvChargerType]
):
"""All valid types in short form."""
str_to_enum = {}

for enum in BatteryType:
short_name = enum.name.split("_")[-1]
str_to_enum[short_name] = enum

for enum in InverterType:
short_name = enum.name.split("_")[-1]
str_to_enum[short_name] = enum

for enum in EvChargerType:
short_name = enum.name.split("_")[-1]
str_to_enum[short_name] = enum

return str_to_enum

short_types = valid_short_types()

# Attempt to parse as component categories with types
try:
return TargetCategoryAndTypes(
[
CategoryAndType(
ComponentCategory[cat.strip().upper()],
short_types[type.strip().upper()]
if type in short_types
else None,
)
for cat, type in (cat.split(":") for cat in values)
]
)
except KeyError as e:
error = e

self.fail(
f'Invalid component category list or ID list: "{value}".\n'
f'Error: "{error}"\n\n'
"Possible categories: BATTERY, GRID, METER, INVERTER, EV_CHARGER, CHP ",
"Valid formats:\n"
"- 1,2,3 # A list of component IDs\n"
"- METER,INVERTER # A list of component categories\n"
"- BATTERY:NA_ION,INVERTER:SOLAR # A list of component categories with types\n"
"Valid component categories:\n"
f"{', '.join([cat.name for cat in ComponentCategory])}\n"
"Valid component types:\n" # using prepared short_types for better readability
f"{', '.join([f'{cat}:{type}' for cat, type in short_types.items()])}",
param,
ctx,
)
Expand Down
7 changes: 5 additions & 2 deletions src/frequenz/client/dispatch/_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@
from .types import (
Dispatch,
DispatchEvent,
TargetCategories,
TargetCategoryAndTypes,
TargetComponents,
TargetIds,
_target_components_to_protobuf,
)

Expand Down Expand Up @@ -98,7 +101,7 @@ async def list(
self,
microgrid_id: int,
*,
target_components: Iterator[TargetComponents] = iter(()),
target_components: TargetComponents,
start_from: datetime | None = None,
start_to: datetime | None = None,
end_from: datetime | None = None,
Expand Down Expand Up @@ -148,7 +151,7 @@ def to_interval(
) -> PBTimeIntervalFilter | None:
return (
PBTimeIntervalFilter(
**{"from": to_timestamp(from_)}, to=to_timestamp(to)
from_time=to_timestamp(from_), to_time=to_timestamp(to)
)
if from_ or to
else None
Expand Down
6 changes: 3 additions & 3 deletions src/frequenz/client/dispatch/recurrence.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ def from_protobuf(cls, pb_criteria: PBRecurrenceRule.EndCriteria) -> "EndCriteri
match pb_criteria.WhichOneof("count_or_until"):
case "count":
instance.count = pb_criteria.count
case "until":
instance.until = to_datetime(pb_criteria.until)
case "until_time":
instance.until = to_datetime(pb_criteria.until_time)
return instance

def to_protobuf(self) -> PBRecurrenceRule.EndCriteria:
Expand All @@ -103,7 +103,7 @@ def to_protobuf(self) -> PBRecurrenceRule.EndCriteria:
if self.count is not None:
pb_criteria.count = self.count
elif self.until is not None:
pb_criteria.until.CopyFrom(to_timestamp(self.until))
pb_criteria.until_time.CopyFrom(to_timestamp(self.until))

return pb_criteria

Expand Down
8 changes: 4 additions & 4 deletions src/frequenz/client/dispatch/test/_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,20 +180,20 @@ def _filter_dispatch(
if target != dispatch.target:
return False
if _filter.HasField("start_time_interval"):
if start_from := _filter.start_time_interval.__dict__["from"]:
if start_from := _filter.start_time_interval.from_time:
if dispatch.start_time < _to_dt(start_from):
return False
if start_to := _filter.start_time_interval.to:
if start_to := _filter.start_time_interval.to_time:
if dispatch.start_time >= _to_dt(start_to):
return False
if _filter.HasField("end_time_interval"):
if end_from := _filter.end_time_interval.__dict__["from"]:
if end_from := _filter.end_time_interval.from_time:
if (
dispatch.duration
and dispatch.start_time + dispatch.duration < _to_dt(end_from)
):
return False
if end_to := _filter.end_time_interval.to:
if end_to := _filter.end_time_interval.to_time:
if (
dispatch.duration
and dispatch.start_time + dispatch.duration >= _to_dt(end_to)
Expand Down
62 changes: 53 additions & 9 deletions src/frequenz/client/dispatch/test/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,17 @@

from .._internal_types import rounded_start_time
from ..recurrence import EndCriteria, Frequency, RecurrenceRule, Weekday
from ..types import Dispatch
from ..types import (
BatteryType,
CategoryAndType,
Dispatch,
EvChargerType,
InverterType,
TargetCategories,
TargetCategoryAndTypes,
TargetComponents,
TargetIds,
)


class DispatchGenerator:
Expand Down Expand Up @@ -66,6 +76,30 @@ def generate_recurrence_rule(self) -> RecurrenceRule:
],
)

def generate_target_cat_and_type(self) -> CategoryAndType:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: This is already pretty long, I wouldn't save the 5 extra chars from egory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're worse than copilot

"""Generate a random category and type.

Returns:
a random category and type
"""
category = self._rng.choice(list(ComponentCategory)[1:])
type = None

match category:
case ComponentCategory.BATTERY:
type = self._rng.choice(list(BatteryType)[1:])
case ComponentCategory.INVERTER:
type = self._rng.choice(list(InverterType)[1:])
case ComponentCategory.EV_CHARGER:
type = self._rng.choice(list(EvChargerType)[1:])
case _:
type = None

return CategoryAndType(
category=category,
type=type,
)

def generate_dispatch(self) -> Dispatch:
"""Generate a random dispatch instance.

Expand Down Expand Up @@ -94,14 +128,24 @@ def generate_dispatch(self) -> Dispatch:
),
target=self._rng.choice( # type: ignore
[
[
self._rng.choice(list(ComponentCategory)[1:])
for _ in range(self._rng.randint(1, 10))
],
[
self._rng.randint(1, 100)
for _ in range(self._rng.randint(1, 10))
],
TargetCategories(
[
self._rng.choice(list(ComponentCategory)[1:])
for _ in range(self._rng.randint(1, 10))
]
),
TargetIds(
[
self._rng.randint(1, 100)
for _ in range(self._rng.randint(1, 10))
]
),
TargetCategoryAndTypes(
[
self.generate_target_cat_and_type()
for _ in range(self._rng.randint(1, 10))
]
),
]
),
active=self._rng.choice([True, False]),
Expand Down
Loading
Loading