Implement a ComponentGraphGenerator from the assets API
#16
Conversation
Add placeholder test until there are actual tests Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
github-actions
bot
added
part:tests
There was a problem hiding this comment.
Pull Request Overview
This PR implements a ComponentGraphGenerator class that generates component graphs for microgrids using the Assets API. The implementation replaces template/boilerplate code with production functionality.
- Adds
ComponentGraphGeneratorclass for fetching and constructing microgrid component graphs from the Assets API - Removes template code (
delete_mefunction) and adds real test coverage for formula generation - Adds git-based dependencies for
frequenz-microgrid-component-graphandfrequenz-client-assets
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/frequenz/gridpool/_graph_generator.py |
New module implementing ComponentGraphGenerator class with graph generation logic |
src/frequenz/gridpool/__init__.py |
Exports ComponentGraphGenerator and fixes docstring typo |
tests/test_gridpool.py |
Replaces template tests with actual test for formula generation using mocked Assets API |
pyproject.toml |
Adds git-based dependencies and removes TODO comments |
Comments suppressed due to low confidence (1)
tests/test_gridpool.py:6
- Import of 'asyncio' is not used.
import asyncio
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| For example: `grpc://localhost:1090?ssl=true`. | ||
| auth_key: The authentication key to use for the connection. | ||
| sign_secret: The secret to use for signing requests. | ||
| channel_defaults: The default options use to create the channel when not |
There was a problem hiding this comment.
Corrected 'use' to 'used' in the documentation.
| channel_defaults: The default options use to create the channel when not | |
| channel_defaults: The default options used to create the channel when not |
| with pytest.raises(RuntimeError, match="This function should be removed!"): | ||
| delete_me(blow_up=True) | ||
| async def test_formula_generation() -> None: | ||
| """Test that the frequenz.gridpool package loads correctly.""" |
There was a problem hiding this comment.
The docstring is misleading. This test validates formula generation from component graphs, not package loading. Consider updating to: 'Test formula generation from component graph.'
| """Test that the frequenz.gridpool package loads correctly.""" | |
| """Test formula generation from component graph.""" |
| dependencies = [ | ||
| "typing-extensions >= 4.14.1, < 5", | ||
| "frequenz-microgrid-component-graph @ git+https://github.com/shsms/frequenz-microgrid-component-graph-python.git@c4a458e06e541846de0a25142d5b821dd7934f47", | ||
| "frequenz-client-assets @ git+https://github.com/shsms/frequenz-client-assets-python@bbf09104df24ddfac497e2a3dd66fe68cfdacd25" |
There was a problem hiding this comment.
[nitpick] Missing trailing comma after the last dependency. For consistency and easier future modifications, add a trailing comma.
| "frequenz-client-assets @ git+https://github.com/shsms/frequenz-client-assets-python@bbf09104df24ddfac497e2a3dd66fe68cfdacd25" | |
| "frequenz-client-assets @ git+https://github.com/shsms/frequenz-client-assets-python@bbf09104df24ddfac497e2a3dd66fe68cfdacd25", |
| if any(c is None for c in connections): | ||
| raise ValueError("Failed to load all electrical component connections.") | ||
|
|
There was a problem hiding this comment.
The None check at line 83 is unnecessary since the type hint indicates list_microgrid_electrical_component_connections returns a list of connections, not optional connections. If None values are genuinely possible, the type hints should reflect this. Consider removing this check or updating the type hints for clarity.
| if any(c is None for c in connections): | |
| raise ValueError("Failed to load all electrical component connections.") |
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
2 participants
No description provided.