Skip to content

Allow multiplying Quantity by float #875

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.

Sign up for GitHub

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 6 commits into from
Feb 15, 2024
Merged

Allow multiplying Quantity by float #875

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f78393f
Allow all quantities multiplication by float | Percentage
llucax Feb 8, 2024
7c754a6
Improve `type: ignore` for `__mul__`
llucax Feb 13, 2024
a4e879a
Use hypothesis for `__mul__` `Percentage` tests
llucax Feb 13, 2024
7eaee36
Add `__mul__()` tests for `float`
llucax Feb 13, 2024
5118985
Update release notes
llucax Feb 13, 2024
9d45a46
Apply suggestions from code review
llucax Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/frequenz/sdk/timeseries/_quantities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,15 @@ def as_megawatts(self) -> float:
"""
return self._base_value / 1e6

@overload # type: ignore
# We need the ignore here because otherwise mypy will give this error:
# > Overloaded operator methods can't have wider argument types in overrides
# The problem seems to be when the other type implements an **incompatible**
# __rmul__ method, which is not the case here, so we should be safe.
# Please see this example:
# https://github.com/python/mypy/blob/c26f1297d4f19d2d1124a30efc97caebb8c28616/test-data/unit/check-overloading.test#L4738C1-L4769C55
# And a discussion in a mypy issue here:
# https://github.com/python/mypy/issues/4985#issuecomment-389692396
@overload # type: ignore[override]
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to add some more context, when Marenz was adding these, we had no idea what the issue was, because without these, mypy kept crashing. Looks like they've fixed that bug and now we are able to understand the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I didn't get a crash (thankfully!) but I still had to do my research, because the error message is not very clear why is there a problem with doing that.

def __mul__(self, scalar: float, /) -> Self:
"""Scale this power by a scalar.

Expand Down Expand Up @@ -758,7 +766,8 @@ def as_milliamperes(self) -> float:
"""
return self._base_value * 1e3

@overload # type: ignore
# See comment for Power.__mul__ for why we need the ignore here.
@overload # type: ignore[override]
def __mul__(self, scalar: float, /) -> Self:
"""Scale this current by a scalar.

Expand Down Expand Up @@ -885,7 +894,8 @@ def as_kilovolts(self) -> float:
"""
return self._base_value / 1e3

@overload # type: ignore
# See comment for Power.__mul__ for why we need the ignore here.
@overload # type: ignore[override]
def __mul__(self, scalar: float, /) -> Self:
"""Scale this voltage by a scalar.

Expand Down