-
Notifications
You must be signed in to change notification settings - Fork 4
Add shared type for all Quantities #22
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
Add shared type for all Quantities #22
Conversation
c8334a7 to
7394d7b
Compare
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.
I'm not sure if I would declare QuantityT here, I think this is only a requirement for the formula engine because of the time when mypy wasn't working and formula engine was added and the type checking was messed up. Ideally I think a TypeVar for quantities should be TypeVar("QuantityT", bound=Quantity), so it resolves to the concrete type, I'm not sure why we had to go with the current version, I think this actually always will resolve to Quantity.
I think for now I would keep this as a hack in the SDK.
|
|
||
| ## Summary | ||
|
|
||
| This is the initial release, extracted from the [SDK v1.0.0rc601](https://github.com/frequenz-floss/frequenz-sdk-python/releases/tag/v1.0.0-rc601). |
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.
I wouldn't remove this until we release v1.0.0, we can add that it is based on that code but with some updates.
| ## New Features | ||
|
|
||
| - Added support for `__round__` (`round(quantity)`), `__pos__` (`+quantity`) and `__mod__` (`quantity % quantity`) operators. | ||
| - Add `QuantityT` type alias for `Quantity` and `QuantityLike` types. |
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.
What is QuantityLike?
|
Just to clarify, I'm not completely against adding it here if it is better/more easy for some reason, just it wouldn't be my first choice for the reasons I mentioned above. |
Signed-off-by: Elzbieta Kotulska <[email protected]>
b4add4e to
3ba8950
Compare
|
As suggested QuantityT was moved back to frequenz-sdk |
No description provided.