`sum`: `Protocol` defining `add` by jpy-git · Pull Request #7578 · python/typeshed · GitHub

Partially addresses #7574.

As suggested by @JelleZijlstra in this comment we should define an __add__ protocol for use in sum to prevent things like

sum([1, None])

In 90% of cases, I think people will probably be using sum to add up variables that are all of the same type, e.g. sum((1.4, 2.3, 3.7, 4.2)). We can safely say for this that the result will be of type float.

So, although strictly speaking you're entirely correct — the return type isn't guaranteed to always be _SumT, far from it! — I nonetheless think I prefer using _SumT over Any for the return type, so that we can preserve precise type inference for the common case.

But I'm curious about what other people think :)

Also, note that the TypeVar is currently unbound in this overload, so either the return type needs to use _SumT instead of Any, or the parameter type needs to change to use _SupportsSum instead of _SumT :)

What if we make the protocol generic over the return type of __add__, and make it sum(Iterable[_SupportsSum[_T]]) -> _T?

Ahh shoot this was actually unintentional, was playing round with the typings and forgot to change it back 😅
I agree that _SumT makes sense, so will change it back for now 👍

What if we make the protocol generic over the return type of __add__, and make it sum(Iterable[_SupportsSum[_T]]) -> _T?

Good shout, will give it a go

@JelleZijlstra, strictly speaking I'm not sure it's that simple, because there's __radd__ to consider, which can change the return type. If Y is a subclass of X, and Y defines __radd__, then Y.__radd__ takes precedence over X.__add__ when evaluating the operation x + y (where x is an instance of X, and y is an instance of Y)

Idk how much that actually matters though

Yes, getting a completely precise return type in the face of mixed types and __radd__ is very complicated. The current version is technically incorrect for the case where two arguments are passed, the iterable is empty, and the default returns an unrelated type from its __add__.

mypy-primer found a lot of new false positives from my suggestion, so we should probably undo that and just go back to assuming that __add__ returns the same type.

Yeah I've reverted back to using TypeVars in the latest commit 😄

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

What happened to the freqtrade one?

(code link: https://github.com/freqtrade/freqtrade/blob/0b40c37cc54dbf97ff8ed1b82330e9d158d91d6b/freqtrade/persistence/models.py#L1204)

What happened to the freqtrade one?

Yes, that one looked like a true positive. I think close_profit_abs attribute is declared here as Optional[float]: https://github.com/freqtrade/freqtrade/blob/0b40c37cc54dbf97ff8ed1b82330e9d158d91d6b/freqtrade/persistence/models.py#L296

What happened to the freqtrade one?

Was just wondering the same thing!

All that's changed vs the first commit is altering the return type

diff --git a/stdlib/builtins.pyi b/stdlib/builtins.pyi
index 9372b2a1..ce2042bc 100644
--- a/stdlib/builtins.pyi
+++ b/stdlib/builtins.pyi
@@ -1485,7 +1485,7 @@ _SumT = TypeVar("_SumT", bound=_SupportsSum)
 _SumS = TypeVar("_SumS", bound=_SupportsSum)
 
 @overload
-def sum(__iterable: Iterable[_SumT]) -> Any | Literal[0]: ...
+def sum(__iterable: Iterable[_SumT]) -> _SumT | Literal[0]: ...
 
 if sys.version_info >= (3, 8):
     @overload

This is very strange, I can't reproduce it on MyPy playground:

from typing import *

class _SupportsSum(Protocol):
    def __add__(self, __x: Any) -> Any: ...

_SumT = TypeVar("_SumT", bound=_SupportsSum)
_SumS = TypeVar("_SumS", bound=_SupportsSum)

@overload  # type: ignore[no-overload-impl]
def sum(__iterable: Iterable[_SumT]) -> _SumT | Literal[0]: ...
@overload
def sum(__iterable: Iterable[_SumT], start: _SumS) -> _SumT | _SumS: ...

it: Iterable[float | None]

sum(it)  # error: Value of type variable "_SumT" of "sum" cannot be "Optional[float]"

Can't reproduce it with the conditional overloads either.

I've triggered a new run of mypy_primer

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

So testing locally these examples work as expected for me

from typing import Iterator, Union


def foo() -> Iterator[int]: ...

def bar() -> Iterator[Union[int, None]]: ...

sum([1, 2])
sum([1, None])
sum(foo())
sum(bar())

test.py:9: error: Value of type variable "_SumT" of "sum" cannot be "Optional[int]"
test.py:11: error: Value of type variable "_SumT" of "sum" cannot be "Optional[int]"

But if I run mypy --custom-typeshed-dir . ../freqtrade/freqtrade/persistence/models.py | grep "models.py:1203" on freqtrade it returns nothing when we have the _SumT return type
However, with the Any return type I get:
../freqtrade/freqtrade/persistence/models.py:1203: error: Value of type variable "_SumT" of "sum" cannot be "Optional[float]" 🤔

I thought type context might help, but this variation that's closer to the freqtrade code doesn't repro it either: https://mypy-play.net/?mypy=latest&python=3.10&flags=strict&gist=3ba27f6c0834c95cfdb5b2d752387275

Okay here's something odd. I've put a reveal_type around the line in question:

total_profit = sum(
                reveal_type(t.close_profit_abs for t in LocalTrade.get_trades_proxy(is_open=False)))

If I run pyright then I get this output:

/Users/joe/repos/freqtrade/freqtrade/persistence/models.py:1204:29 - information: Type of "t.close_profit_abs for t in LocalTrade.get_trades_proxy(is_open=False)" is "Generator[float | None, None, None]"
  /Users/joe/repos/freqtrade/freqtrade/persistence/models.py:1204:17 - error: Argument of type "Generator[float | None, None, None]" cannot be assigned to parameter "__iterable" of type "Iterable[_SumT@sum]" in function "sum"

All works as expected

However, with mypy:

../freqtrade/freqtrade/persistence/models.py:1204: note: Revealed type is "typing.Generator[Union[builtins.float, None], None, None]"
../freqtrade/freqtrade/persistence/models.py:1204: note: Revealed type is "typing.Generator[Any, None, None]"

No error raised and 2 revealed types?? Seems like that second one could be causing the error to get ignored?

And when I use the Any return type I get:

../freqtrade/freqtrade/persistence/models.py:1203: error: Value of type variable "_SumT" of "sum" cannot be "Optional[float]"
../freqtrade/freqtrade/persistence/models.py:1204: note: Revealed type is "typing.Generator[Union[builtins.float, None], None, None]"

error raised and only 1 revealed type

So is this a mypy specific issue?

So is this a mypy specific issue?

I think so, almost certainly :) We should probably merge this as it is, if @JelleZijlstra is in agreement with me. It's just very strange that we can't reproduce it in miniature.

The "double output for reveal_type" thing is similar to python/mypy#10739, which might give us a clue.

Something else that's interesting: this PR has also exposed a (completely unrelated) pyright bug, I think. The first commit, where the TypeVar was unbound in the first overload, should have triggered pyright's reportInvalidTypeVarUse check. So we really hit the jackpot here!

Yes, I think we should merge.

I tried some other variations in mypy playground but couldn't get the double reveal type. Normally that happens with TypeVars with constraints, but there aren't any here (or in the issue Alex linked above). Not sure what's happening there.

Thanks!

I think I know what's going on with freqtrade. The change in this PR that silenced mypy_primer was:

diff --git a/stdlib/builtins.pyi b/stdlib/builtins.pyi
index 9372b2a1..ce2042bc 100644
--- a/stdlib/builtins.pyi
+++ b/stdlib/builtins.pyi
@@ -1485,7 +1485,7 @@ _SumT = TypeVar("_SumT", bound=_SupportsSum)
 _SumS = TypeVar("_SumS", bound=_SupportsSum)
 
 @overload
-def sum(__iterable: Iterable[_SumT]) -> Any | Literal[0]: ...
+def sum(__iterable: Iterable[_SumT]) -> _SumT | Literal[0]: ...
 
 if sys.version_info >= (3, 8):
     @overload

In freqtrade, the result of sum(...) is assigned to total_profit, which has type Any. Previously mypy tried _SumT = Optional[float] when solving the TypeVars, and that caused the error. But now that _SumT is also used in the return type, and the result is assigned to Any, it doesn't give up there. It instead tries _SumT = Any, which succeeds and silences the error. You can see the two tries in the reveal_type() output, too.

Thanks @Akuli, I missed that the variable was of type Any. I can reproduce the double reveal_type() now: https://mypy-play.net/?mypy=latest&python=3.10&flags=strict&gist=4b96aaf8e1261db4bf484c0f76138638 (though interestingly only with overloads involved).

sum: Protocol defining __add__

f41f186

AlexWaygood reviewed Apr 1, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

Make Protocol generic

810250b

This comment has been minimized.

Sign in to view

Revert to TypeVars

ae316c3

This comment has been minimized.

Sign in to view

AlexWaygood approved these changes Apr 1, 2022

View reviewed changes

AlexWaygood merged commit d45c908 into python:master Apr 1, 2022

AlexWaygood mentioned this pull request Apr 1, 2022

Unbound TypeVar apparently undetected in function overload microsoft/pyright#3288

Closed

jpy-git deleted the sum_protocol branch April 1, 2022 22:57

JelleZijlstra mentioned this pull request Apr 2, 2022

Sum of an iterable of bools is considered to have a bool type #7579

Closed

davidism mentioned this pull request Apr 28, 2022

Document typeshed better: a mypy upgrade suddenly triggers new errors due to typeshed changes #7735

Closed

kaushikcfd mentioned this pull request Apr 29, 2022

builtins.sum: Spurious error for operands having __add__ defined using partialmethod #7739

Closed

cake-monotone mentioned this pull request Mar 2, 2025

Improving type support for math.prod #13572

Merged

`sum`: `Protocol` defining `__add__` by jpy-git · Pull Request #7578 · python/typeshed · GitHub

Repository navigation

sum: Protocol defining __add__ - #7578

sum: Protocol defining __add__#7578AlexWaygood merged 3 commits intopython:masterpython/typeshed:masterfrom jpy-git:sum_protocoljpy-git/typeshed:sum_protocolCopy head branch name to clipboard

Conversation

jpy-git commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

JelleZijlstra commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jpy-git commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Apr 1, 2022

Uh oh!

github-actions Bot commented Apr 1, 2022

Uh oh!

jpy-git commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra commented Apr 1, 2022

Uh oh!

jpy-git commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Apr 1, 2022

Uh oh!

AlexWaygood commented Apr 1, 2022

Uh oh!

JelleZijlstra commented Apr 1, 2022

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Akuli commented Apr 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra commented Apr 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`sum`: `Protocol` defining `add` by jpy-git · Pull Request #7578 · python/typeshed · GitHub

sum: Protocol defining add - #7578

sum: Protocol defining add#7578
AlexWaygood merged 3 commits into
python:masterfrom
jpy-git:sum_protocol

jpy-git commented Apr 1, 2022 •
edited
Loading

AlexWaygood Apr 1, 2022 •
edited
Loading

JelleZijlstra commented Apr 1, 2022 •
edited
Loading

AlexWaygood commented Apr 1, 2022 •
edited
Loading

jpy-git commented Apr 1, 2022 •
edited
Loading

AlexWaygood commented Apr 1, 2022 •
edited
Loading

jpy-git commented Apr 1, 2022 •
edited
Loading

jpy-git commented Apr 1, 2022 •
edited
Loading

Akuli commented Apr 2, 2022 •
edited
Loading