-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
More improvements to pow
stubs
#7737
Conversation
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core)
+ homeassistant/util/color.py:300: error: Redundant cast to "float" [redundant-cast]
|
I don't understand the primer output. The cast is |
Oh interesting. I don't think |
Here's a self-contained version: from typing import cast
def color_xy_brightness_to_RGB(
vX: float, vY: float, ibrightness: int
) -> tuple[int, int, int]:
"""Convert from XYZ to RGB."""
brightness = ibrightness / 255.0
if brightness == 0.0:
return (0, 0, 0)
Y = brightness
if vY == 0.0:
vY += 0.00000000001
X = (Y / vY) * vX
Z = (Y / vY) * (1 - vX - vY)
# Convert to RGB using Wide RGB D65 conversion.
r = X * 1.656492 - Y * 0.354851 - Z * 0.255038
g = -X * 0.707196 + Y * 1.655397 + Z * 0.036152
b = X * 0.051713 - Y * 0.121364 + Z * 1.011530
# Apply reverse gamma correction.
r, g, b = map(
lambda x: (12.92 * x)
if (x <= 0.0031308)
else ((1.0 + 0.055) * cast(float, pow(x, (1.0 / 2.4))) - 0.055),
[r, g, b],
)
# Bring all negative components to zero.
r, g, b = map(lambda x: max(0, x), [r, g, b])
# If one component is greater than 1, weight components by that value.
max_component = max(r, g, b)
if max_component > 1:
r, g, b = map(lambda x: x / max_component, [r, g, b])
ir, ig, ib = map(lambda x: int(x * 255), [r, g, b])
return (ir, ig, ib) It generates the same mypy error as in the primer output. But the cast is in fact not redundant. If you remove it, you get a new error: Also, if you replace
I'm not yet sure about what's going on and where |
And here's a smaller reproducer: from typing import cast
def with_cast() -> None:
a, b = map(
lambda x: cast(float, pow(x, (1.0 / 2.4))),
[1.2, 3.4],
)
def without_cast() -> None:
a, b = map(
lambda x: pow(x, (1.0 / 2.4)),
[1.2, 3.4],
) The |
And now that I compare the output of tl;dr I created a long wall of text that can be ignored :D |
Agreed :) |
@AlexWaygood I don't get it. Why not declare the return type to be |
We have a general, longstanding policy against using union return types, in the name of preferring "false negative" errors over "false positive" errors. More on that here: python/mypy#1693. It's reasonable to discuss whether that should continue to be typeshed's policy in 2022. But that discussion should happen in an issue rather than a PR that was merged five months ago :) Also, note that proposals to use union return types for Here's another PR where I made some changes to the |
Literal[0]
int.__pow__
overload into two so that the tests intest_pow.py
pass mypy.float.__rpow__
and two overloads tobuiltins.pow()
using the good-old_PositiveInteger
/_NegativeInteger
hack.Resolves #7733.
@TylerYep, this doesn't fix the problem you raised in a general way, but doing so is impossible unfortunately, since there's no general way (in the current type system) of mypy distinguishing between positive arguments and negative arguments. Returning
Any
is sometimes the least-bad option, which is why the third overload infloat.__rpow__
still has to returnAny
.