From 481d99d5d7d1e49d8cce00298927592cec8b0604 Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Mon, 3 Feb 2025 17:54:14 +0100 Subject: [PATCH 1/6] Support listing orders without price or quantity Cancelled orders don't have a price or quantity, which leads to failures when converting from protobuf. This allows missing price and quantities. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- src/frequenz/client/electricity_trading/_types.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/electricity_trading/_types.py b/src/frequenz/client/electricity_trading/_types.py index 409dcdb2..d608d33d 100644 --- a/src/frequenz/client/electricity_trading/_types.py +++ b/src/frequenz/client/electricity_trading/_types.py @@ -1010,8 +1010,16 @@ def from_pb(cls, order: electricity_trading_pb2.Order) -> Self: delivery_period=DeliveryPeriod.from_pb(order.delivery_period), type=OrderType.from_pb(order.type), side=MarketSide.from_pb(order.side), - price=Price.from_pb(order.price), - quantity=Power.from_pb(order.quantity), + price=( + Price.from_pb(order.price) + if order.HasField("price") + else Price(Decimal("NaN"), Currency.UNSPECIFIED) + ), + quantity=( + Power.from_pb(order.quantity) + if order.HasField("quantity") + else Power(Decimal("NaN")) + ), stop_price=( Price.from_pb(order.stop_price) if order.HasField("stop_price") From 139a7ba2f01444d94fd1f82439247553435be441 Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Mon, 3 Feb 2025 20:35:04 +0100 Subject: [PATCH 2/6] Ensure price and quantity exist in non-cancelled orders Cancelled orders may not have a price or quantity, all other orders must have these set. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- .../client/electricity_trading/_types.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/frequenz/client/electricity_trading/_types.py b/src/frequenz/client/electricity_trading/_types.py index d608d33d..a01745df 100644 --- a/src/frequenz/client/electricity_trading/_types.py +++ b/src/frequenz/client/electricity_trading/_types.py @@ -1316,8 +1316,11 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self: Returns: OrderDetail object corresponding to the protobuf message. + + Raises: + ValueError: If the order price and quantity are not specified for a non-canceled order. """ - return cls( + od = cls( order_id=order_detail.order_id, order=Order.from_pb(order_detail.order), state_detail=StateDetail.from_pb(order_detail.state_detail), @@ -1329,6 +1332,19 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self: ), ) + # Only cancelled orders are allowed to have missing price or quantity + missing_price_or_quantity = ( + od.order.price.amount.is_nan() + or od.order.price.currency == Currency.UNSPECIFIED + or od.order.quantity.mw.is_nan() + ) + if missing_price_or_quantity and od.state_detail.state != OrderState.CANCELED: + raise ValueError( + f"Price and quantity must be specified for a non-canceled order (`{od}`)." + ) + + return od + def to_pb(self) -> electricity_trading_pb2.OrderDetail: """Convert an OrderDetail object to protobuf OrderDetail. From 023f844cb70b4dd2d80fcd9b6545534b74e65b01 Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Tue, 4 Feb 2025 11:08:31 +0100 Subject: [PATCH 3/6] Add test for missing fields in order detail Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- tests/test_types.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_types.py b/tests/test_types.py index e30d6c6f..f216e78b 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -498,6 +498,29 @@ def test_order_detail_from_pb() -> None: ) +def test_order_detail_from_pb_missing_fields() -> None: + """Test the client order detail type conversion from protobuf with missing fields.""" + # Missing price + od_pb1 = electricity_trading_pb2.OrderDetail() + od_pb1.CopyFrom(ORDER_DETAIL_PB) + od_pb1.order.ClearField("price") + # Not allowed for active orders + with pytest.raises(ValueError): + OrderDetail.from_pb(od_pb1) + # But allowed for canceled orders + od_pb1.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED + OrderDetail.from_pb(od_pb1) + + # Missing quantity (same logic as above) + od_pb2 = electricity_trading_pb2.OrderDetail() + od_pb2.CopyFrom(ORDER_DETAIL_PB) + od_pb2.order.ClearField("quantity") + with pytest.raises(ValueError): + OrderDetail.from_pb(od_pb2) + od_pb2.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED + OrderDetail.from_pb(od_pb2) + + def test_order_detail_no_timezone_error() -> None: """Test that an order detail with inputs with no timezone raises a ValueError.""" with pytest.raises(ValueError): From 9373c6456fee5d3d953e6d61a55c716b85893c0e Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Mon, 3 Feb 2025 19:41:19 +0100 Subject: [PATCH 4/6] Write header when receiving orders with CLI tool Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- src/frequenz/client/electricity_trading/cli/etrading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frequenz/client/electricity_trading/cli/etrading.py b/src/frequenz/client/electricity_trading/cli/etrading.py index df1adf7f..fe7523ef 100644 --- a/src/frequenz/client/electricity_trading/cli/etrading.py +++ b/src/frequenz/client/electricity_trading/cli/etrading.py @@ -97,7 +97,7 @@ async def list_orders( """ client = Client(server_url=url, auth_key=key) - # print_header() + print_order_header() delivery_period = None # If delivery period is selected, list historical orders also From 6727b65827fa02f27c4f9d02a997b3fb802797dc Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Mon, 3 Feb 2025 19:49:53 +0100 Subject: [PATCH 5/6] Fix headers for trade and order listing in CLI tool Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- .../electricity_trading/cli/etrading.py | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/frequenz/client/electricity_trading/cli/etrading.py b/src/frequenz/client/electricity_trading/cli/etrading.py index fe7523ef..f6369633 100644 --- a/src/frequenz/client/electricity_trading/cli/etrading.py +++ b/src/frequenz/client/electricity_trading/cli/etrading.py @@ -200,18 +200,18 @@ async def cancel_order( def print_trade_header() -> None: """Print trade header in CSV format.""" header = ( - "public_trade_id, " - "execution_time, " - "delivery_period_start, " - "delivery_period_duration, " - "buy_delivery_area_code, " - "sell_delivery_area_code, " - "buy_delivery_area_code_type, " - "sell_delivery_area_code_type" - "quantity_mw, " - "price, " - "currency, " - "state, " + "public_trade_id," + "execution_time," + "delivery_period_start," + "delivery_period_duration," + "buy_delivery_area_code," + "sell_delivery_area_code," + "buy_delivery_area_code_type," + "sell_delivery_area_code_type," + "quantity_mw," + "currency," + "price," + "state " ) print(header) @@ -238,20 +238,20 @@ def print_trade(trade: PublicTrade) -> None: def print_order_header() -> None: """Print order header in CSV format.""" header = ( - "order_id, " - "create_time, " - "modification_time, " - "delivery_period_start, " - "delivery_period_duration" - "delivery_area_code, " - "delivery_area_code_type, " - "order_type, " - "quantity_mw, " - "open_quantity_mw, " - "side, " - "currency, " - "price, " - "state, " + "order_id," + "create_time," + "modification_time," + "delivery_period_start," + "delivery_period_duration," + "delivery_area_code," + "delivery_area_code_type," + "order_type," + "quantity_mw," + "open_quantity_mw," + "side," + "currency," + "price," + "state" ) print(header) From d28274c79404fc21e5d66c473fd8e2a728245b58 Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Mon, 3 Feb 2025 20:45:16 +0100 Subject: [PATCH 6/6] Update release notes Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- RELEASE_NOTES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c6f78be8..737ec87a 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -16,4 +16,6 @@ ## Bug Fixes +* Deal correctly with cancelled orders with no price or quantity +