-
Couldn't load subscription status.
- Fork 5
Add ID wrapper classes #122
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ca81c50
Sort exported symbols
llucax ec1b472
Use `Iterator` from `collections.abs`
llucax 51128dd
tests: Import from public modules when possible
llucax 88ad8dc
Add strongly typed IDs for microgrids and components
llucax cab70f9
Use the new ID types throughout the client
llucax b38bb9b
Add metadata initialization tests
llucax b6dbc6a
Update release notes
llucax File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| # License: MIT | ||
| # Copyright © 2025 Frequenz Energy-as-a-Service GmbH | ||
|
|
||
| """Strongly typed IDs for microgrids and components.""" | ||
|
|
||
|
|
||
| class MicrogridId: | ||
| """A unique identifier for a microgrid.""" | ||
|
|
||
| def __init__(self, id_: int, /) -> None: | ||
| """Initialize this instance. | ||
|
|
||
| Args: | ||
| id_: The numeric unique identifier of the microgrid. | ||
|
|
||
| Raises: | ||
| ValueError: If the ID is negative. | ||
| """ | ||
| if id_ < 0: | ||
| raise ValueError("Microgrid ID can't be negative.") | ||
| self._id = id_ | ||
|
|
||
| def __int__(self) -> int: | ||
| """Return the numeric ID of this instance.""" | ||
| return self._id | ||
|
|
||
| def __eq__(self, other: object) -> bool: | ||
| """Check if this instance is equal to another object.""" | ||
| # This is not an unidiomatic typecheck, that's an odd name for the check. | ||
| # isinstance() returns True for subclasses, which is not what we want here. | ||
| # pylint: disable-next=unidiomatic-typecheck | ||
| return type(other) is MicrogridId and self._id == other._id | ||
|
|
||
| def __lt__(self, other: object) -> bool: | ||
| """Check if this instance is less than another object.""" | ||
| # pylint: disable-next=unidiomatic-typecheck | ||
| if type(other) is MicrogridId: | ||
| return self._id < other._id | ||
| return NotImplemented | ||
|
|
||
| def __hash__(self) -> int: | ||
| """Return the hash of this instance.""" | ||
| # We include the class because we explicitly want to avoid the same ID to give | ||
| # the same hash for different classes of IDs | ||
| return hash((MicrogridId, self._id)) | ||
|
|
||
| def __repr__(self) -> str: | ||
| """Return the string representation of this instance.""" | ||
| return f"{type(self).__name__}({self._id!r})" | ||
|
|
||
| def __str__(self) -> str: | ||
| """Return the short string representation of this instance.""" | ||
| return f"MID{self._id}" | ||
|
|
||
|
|
||
| class ComponentId: | ||
| """A unique identifier for a microgrid component.""" | ||
|
|
||
| def __init__(self, id_: int, /) -> None: | ||
| """Initialize this instance. | ||
|
|
||
| Args: | ||
| id_: The numeric unique identifier of the microgrid component. | ||
|
|
||
| Raises: | ||
| ValueError: If the ID is negative. | ||
| """ | ||
| if id_ < 0: | ||
| raise ValueError("Component ID can't be negative.") | ||
| self._id = id_ | ||
|
|
||
| def __int__(self) -> int: | ||
| """Return the numeric ID of this instance.""" | ||
| return self._id | ||
|
|
||
| def __eq__(self, other: object) -> bool: | ||
| """Check if this instance is equal to another object.""" | ||
| # This is not an unidiomatic typecheck, that's an odd name for the check. | ||
| # isinstance() returns True for subclasses, which is not what we want here. | ||
| # pylint: disable-next=unidiomatic-typecheck | ||
| return type(other) is ComponentId and self._id == other._id | ||
|
|
||
| def __lt__(self, other: object) -> bool: | ||
| """Check if this instance is less than another object.""" | ||
| # pylint: disable-next=unidiomatic-typecheck | ||
| if type(other) is ComponentId: | ||
| return self._id < other._id | ||
| return NotImplemented | ||
|
|
||
| def __hash__(self) -> int: | ||
| """Return the hash of this instance.""" | ||
| # We include the class because we explicitly want to avoid the same ID to give | ||
| # the same hash for different classes of IDs | ||
| return hash((ComponentId, self._id)) | ||
|
|
||
| def __repr__(self) -> str: | ||
| """Return the string representation of this instance.""" | ||
| return f"{type(self).__name__}({self._id!r})" | ||
|
|
||
| def __str__(self) -> str: | ||
| """Return the short string representation of this instance.""" | ||
| return f"CID{self._id}" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| # License: MIT | ||
| # Copyright © 2025 Frequenz Energy-as-a-Service GmbH | ||
|
|
||
| """Tests for the microgrid and component IDs.""" | ||
|
|
||
| from dataclasses import dataclass | ||
|
|
||
| import pytest | ||
|
|
||
| from frequenz.client.microgrid import ComponentId, MicrogridId | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class IdTypeInfo: | ||
| """Information about an ID type for testing.""" | ||
|
|
||
| id_class: type | ||
| str_prefix: str | ||
| error_prefix: str | ||
|
|
||
|
|
||
| # Define all ID types to test here | ||
| ID_TYPES: list[IdTypeInfo] = [ | ||
| IdTypeInfo(MicrogridId, "MID", "Microgrid"), | ||
| IdTypeInfo(ComponentId, "CID", "Component"), | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "type_info", | ||
| ID_TYPES, | ||
| ids=lambda type_info: type_info.id_class.__name__, | ||
| ) | ||
| class TestIds: | ||
| """Tests for ID classes.""" | ||
|
|
||
| def test_valid_id(self, type_info: IdTypeInfo) -> None: | ||
| """Test creating a valid ID.""" | ||
| id_obj = type_info.id_class(42) | ||
| assert int(id_obj) == 42 | ||
|
|
||
| def test_negative_id_raises(self, type_info: IdTypeInfo) -> None: | ||
| """Test that creating a negative ID raises ValueError.""" | ||
| error_msg = f"{type_info.error_prefix} ID can't be negative" | ||
| with pytest.raises(ValueError, match=error_msg): | ||
| type_info.id_class(-1) | ||
|
|
||
| def test_equality(self, type_info: IdTypeInfo) -> None: | ||
| """Test equality comparison.""" | ||
| assert type_info.id_class(1) == type_info.id_class(1) | ||
| assert type_info.id_class(1) != type_info.id_class(2) | ||
|
|
||
| # Test against all other types | ||
| for other_type in ID_TYPES: | ||
| if other_type != type_info: | ||
| assert type_info.id_class(1) != other_type.id_class(1) | ||
|
|
||
| def test_ordering(self, type_info: IdTypeInfo) -> None: | ||
| """Test ordering comparison.""" | ||
| assert type_info.id_class(1) < type_info.id_class(2) | ||
| assert not type_info.id_class(2) < type_info.id_class(1) | ||
|
|
||
| # Test against all other types | ||
| for other_type in ID_TYPES: | ||
| if other_type != type_info: | ||
| with pytest.raises(TypeError): | ||
| _ = type_info.id_class(1) < other_type.id_class(2) | ||
|
|
||
| def test_hash(self, type_info: IdTypeInfo) -> None: | ||
| """Test hash behavior.""" | ||
| # Same IDs should hash to same value | ||
| assert hash(type_info.id_class(1)) == hash(type_info.id_class(1)) | ||
| # Different IDs should hash to different values | ||
| assert hash(type_info.id_class(1)) != hash(type_info.id_class(2)) | ||
|
|
||
| # Test against all other types | ||
| for other_type in ID_TYPES: | ||
| if other_type != type_info: | ||
| # Same ID but different types should hash to different values | ||
| assert hash(type_info.id_class(1)) != hash(other_type.id_class(1)) | ||
|
|
||
| def test_str_and_repr(self, type_info: IdTypeInfo) -> None: | ||
| """Test string representations.""" | ||
| id_obj = type_info.id_class(42) | ||
| assert str(id_obj) == f"{type_info.str_prefix}42" | ||
| assert repr(id_obj) == f"{type_info.id_class.__name__}(42)" | ||
|
|
||
| def test_invalid_creation(self, type_info: IdTypeInfo) -> None: | ||
| """Test that creating an ID with a non-integer raises TypeError.""" | ||
| with pytest.raises(TypeError): | ||
| type_info.id_class("not-an-int") |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can still do
isinstanceand disable inheritance for the class using thetyping.finaldecorator.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, not sure if
typing.finalalso performs a runtime check or not, if it doesn't, then the current code is safer as it will be checked at runtime. In practice I don't think it should be a big deal anyway, I doubt anyone would want to subclass these IDs, but in any case addingtyping.final()might be nice addition to make the intention clearer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.python.org/3.11/library/typing.html#typing.final
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.