Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Jun 6, 2025

The new BaseId class provides strongly-typed unique identifiers for entities. This class can be subclassed to create distinct ID types for different components or concepts within a system.

These IDs ensure type safety, meaning that an ID for one type of entity (e.g., a sensor) cannot be mistakenly used where an ID for another type (e.g., a microgrid) is expected.

@Copilot Copilot AI review requested due to automatic review settings June 6, 2025 11:10
@llucax llucax requested a review from a team as a code owner June 6, 2025 11:10
@llucax llucax requested a review from shsms June 6, 2025 11:10
@github-actions github-actions bot added the part:tests Affects the unit, integration and performance (benchmarks) tests label Jun 6, 2025
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jun 6, 2025
@llucax llucax self-assigned this Jun 6, 2025
@llucax llucax added the type:enhancement New feature or enhancement visitble to users label Jun 6, 2025
@llucax llucax added this to the v1.0.0 milestone Jun 6, 2025
@llucax
Copy link
Contributor Author

llucax commented Jun 6, 2025

This was already reviewed in frequenz-floss/frequenz-client-microgrid-python@6d494e1.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new BaseId class to provide strongly typed, system-wide unique identifiers and accompanying tests.

  • Introduces BaseId with subclassing guards for unique string prefixes and optional name checks.
  • Implements comparison, hashing, and string/repr behavior for IDs.
  • Adds tests covering instantiation, equality, ordering, hashing, and string representations.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/test_id.py New tests for BaseId subclasses covering valid IDs, errors, comparison, hashing, and repr/str.
src/frequenz/core/id.py Implements BaseId with __init_subclass__ checks, rich comparison, hashing, and string formatting.
Comments suppressed due to low confidence (4)

tests/test_id.py:69

  • Add a test case to verify that defining two subclasses with the same str_prefix raises a ValueError, covering the uniqueness check in BaseId.__init_subclass__.
    assert repr(id_obj) == "_TestId(42)"

tests/test_id.py:69

  • Add a test to confirm that instantiating BaseId directly raises a TypeError, covering the guard in BaseId.__new__.
    assert repr(id_obj) == "_TestId(42)"

tests/test_id.py:14

  • Add a test that defining a subclass without allow_custom_name=True and with a name not ending in 'Id' raises a TypeError, covering the naming enforcement in __init_subclass__.
class _TestId(BaseId, str_prefix="TEST_ID"):

src/frequenz/core/id.py:68

  • [nitpick] Consider decorating BaseId with @functools.total_ordering so that all comparison methods (__le__, __gt__, __ge__) are automatically provided based on __eq__ and __lt__.
class BaseId:

shsms
shsms previously approved these changes Jun 6, 2025
@llucax llucax added this pull request to the merge queue Jun 6, 2025
@llucax llucax removed this pull request from the merge queue due to a manual request Jun 6, 2025
The new `BaseId` class provides strongly-typed unique identifiers for
entities. This class can be subclassed to create distinct ID types for
different components or concepts within a system.

These IDs ensure type safety, meaning that an ID for one type of entity
(e.g., a sensor) cannot be mistakenly used where an ID for another type
(e.g., a microgrid) is expected.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax added this pull request to the merge queue Jun 6, 2025
Merged via the queue into frequenz-floss:v1.x.x with commit 86add0e Jun 6, 2025
5 checks passed
@llucax llucax deleted the base-id branch June 6, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:tests Affects the unit, integration and performance (benchmarks) tests type:enhancement New feature or enhancement visitble to users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants