Skip to content

Conversation

@ela-kotulska-frequenz
Copy link
Contributor

Remove current quantities definition

@ela-kotulska-frequenz ela-kotulska-frequenz added the type:tech-debt Improves the project without visible changes for users label Oct 14, 2024
@ela-kotulska-frequenz ela-kotulska-frequenz self-assigned this Oct 14, 2024
@ela-kotulska-frequenz ela-kotulska-frequenz requested a review from a team as a code owner October 14, 2024 06:17
@ela-kotulska-frequenz ela-kotulska-frequenz requested review from llucax and removed request for a team October 14, 2024 06:17
@github-actions github-actions bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) part:data-pipeline Affects the data pipeline part:microgrid Affects the interactions with the microgrid labels Oct 14, 2024
Copy link
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

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

Really nice to see this happening!

RELEASE_NOTES.md Outdated
## Upgrading

<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
- Replace `frequenz.sdk.timeseries._quantities` module with the external `frequenz.quantities` package.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not use internal module names as this is intended for end-users, and also give a link to the new library.

Suggested change
- Replace `frequenz.sdk.timeseries._quantities` module with the external `frequenz.quantities` package.
- Replace `Quantity` and its sub-classes (`Power`, `Current`, etc.) in the `frequenz.sdk.timeseries` module with the external [`frequenz-quantities`](https://pypi.org/project/frequenz-quantities/) package. Please add the new library as a dependency and adapt your imports if you are using these types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines 37 to 40
from frequenz.quantities import (
Current,
Energy,
Frequency,
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see you are re-exporting here. I wouldn't do this, I would just go ahead and remove these from here completely (including __all__ of course).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@llucax llucax linked an issue Oct 14, 2024 that may be closed by this pull request
3 tasks
Signed-off-by: Elzbieta Kotulska <[email protected]>
@ela-kotulska-frequenz ela-kotulska-frequenz force-pushed the quantity branch 2 times, most recently from 0e72bcc to 5381c39 Compare October 16, 2024 06:47
Signed-off-by: Elzbieta Kotulska <[email protected]>
@ela-kotulska-frequenz ela-kotulska-frequenz added this pull request to the merge queue Oct 16, 2024
Merged via the queue into frequenz-floss:v1.x.x with commit 8d9e735 Oct 16, 2024
18 checks passed
@ela-kotulska-frequenz ela-kotulska-frequenz deleted the quantity branch October 16, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:data-pipeline Affects the data pipeline part:docs Affects the documentation part:microgrid Affects the interactions with the microgrid part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) type:tech-debt Improves the project without visible changes for users

Projects

Development

Successfully merging this pull request may close these issues.

Split the Quantitys to its own repository

3 participants