Skip to content
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

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

Open
davidism opened this issue Apr 28, 2022 · 9 comments
Labels
project: infrastructure typeshed build, test, documentation, or distribution related

Comments

@davidism
Copy link

davidism commented Apr 28, 2022

I just updated MyPy in Jinja, and had to ignore the return type of sum in one of my functions because MyPy started saying it wasn't compatible with the TypeVar I was using. pallets/jinja@a24df26

I see a recent change related to the return type of sum: #7578. Is this a bug in typeshed/mypy, or is there a better way I should be annotating that function in Jinja?

V = t.TypeVar("V")

def sync_do_sum(
    environment: "Environment",
    iterable: "t.Iterable[V]",
    attribute: t.Optional[t.Union[str, int]] = None,
    start: V = 0,  # type: ignore
) -> V:
    if attribute is not None:
        iterable = map(make_attrgetter(environment, attribute), iterable)

    return sum(iterable, start)  # type: ignore[no-any-return, call-overload]
src/jinja2/filters.py:1295: error: Returning Any from function declared to return "V"  [no-any-return]
src/jinja2/filters.py:1295: error: No overload variant of "sum" matches argument types "Iterable[V]", "V"  [call-overload]
src/jinja2/filters.py:1295: note: Possible overload variants:
src/jinja2/filters.py:1295: note:     def [_SumT <: _SupportsSum] sum(Iterable[_SumT]) -> Union[_SumT, Literal[0]]
src/jinja2/filters.py:1295: note:     def [_SumT <: _SupportsSum, _SumS <: _SupportsSum] sum(Iterable[_SumT], _SumS) -> Union[_SumT, _SumS]
Found 2 errors in 1 file (checked 25 source files)
@srittau
Copy link
Collaborator

srittau commented Apr 28, 2022

The "correct" way to do this is to add a your own protocol and constrain V to that:

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

V = TypeVar("V", bound=_SupportsSum)

But we should probably add SupportsSum to _typeshed to make it a bit easier to use (and to not add it to the global namespace).

@AlexWaygood
Copy link
Member

But we should probably add SupportsSum to _typeshed to make it a bit easier to use (and to not add it to the global namespace).

I'd prefer SupportsAdd as a name, but I agree.

@davidism
Copy link
Author

Is there any way to make this type of error more discoverable? I don't think most people would know to look at typeshed for this error. I wouldn't have known to use something from _typeshed unless I had posted here, I regularly forget that typeshed exposes some types, because it's not mentioned in Python's typing documentation or in mypy/typing-extensions/mypy-extensions.

@davidism
Copy link
Author

If anyone involved in typing is going to be at PyCon sprints, maybe some documentation for typeshed would be a good sprint task.

@srittau
Copy link
Collaborator

srittau commented Apr 28, 2022

The typing documentation is distributed over multiple sites, at the moment. There's also a lot of institutional knowledge that's onlyt written down in some issue discussions. https://typing.readthedocs.io is supposed to become a central documentation repository in the future, but there's nothing really there yet, unfortunately.

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 28, 2022

If anyone involved in typing is going to be at PyCon sprints, maybe some documentation for typeshed would be a good sprint task.

There's actually a nice mypy PR I reviewed the other day that would improve the situation a little bit:

The author doesn't seem to be particularly active at the moment, though, so idk if it has a chance of getting merged.

And I agree with @srittau that it's much better to have this information in a centralised place rather than the mypy docs, and https://typing.readthedocs.io/ seems like the best place for this kind of thing.

@JelleZijlstra
Copy link
Member

It would also be useful to distribute types like SupportsAdd in a .py package, maybe typing-extensions.

@srittau
Copy link
Collaborator

srittau commented Apr 28, 2022

It's been my plan for a long time now to get some of the definition from _typeshed into typing_extensions. (Maybe under typing_extensions.ext?). But time is finite. :(

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 30, 2022

Something like this would also be helpful for guiding people to the likely source of the error in cases like this:

@AlexWaygood AlexWaygood changed the title handling of sum return type changed Jun 29, 2022
@srittau srittau added project: infrastructure typeshed build, test, documentation, or distribution related and removed docs labels Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: infrastructure typeshed build, test, documentation, or distribution related
4 participants