8000 Issue with urlencode typeshed annotation starting in mypy 0.780 · Issue #4234 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Issue with urlencode typeshed annotation starting in mypy 0.780 #4234

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

Jump to bottom
Closed
nipunn1313 opened this issue Jun 17, 2020 · 8 comments · Fixed by #13815
Closed

Issue with urlencode typeshed annotation starting in mypy 0.780 #4234

nipunn1313 opened this issue Jun 17, 2020 · 8 comments · Fixed by #13815
Labels
stubs: false positive Type checkers report false errors

Comments

@nipunn1313
Copy link
Contributor

https://mypy-play.net/?mypy=0.780&python=3.8&gist=67acdcb73a9d1fef5db0780b9a51d9c9

Short repro

from urllib.parse import urlencode, quote

args_dict = {'a': 'b'}
urlencode(args_dict, quote_via=quote)

Passes on 0.770 fails on 0.780

main.py:4: error: Value of type variable "AnyStr" of "urlencode" cannot be "Union[bytes, str]"
Found 1 error in 1 file (checked 1 source file)
@hauntsaninja
Copy link
Collaborator

There don't look to have been any relevant typeshed changes. git bisect tells me the issue was introduced by python/mypy#8167 (the commit message mentions it expects some things to break), so possibly be worth opening on mypy.

The usage of AnyStr type var looks reasonable to me (using Union or something would be looser than we want), but maybe something needs to be marked contravariant. I'll take a look when I'm more awake and let you know whether I think that commit introduced a mypy bug or if the stub was slightly wrong all along and now we know...

@srittau
Copy link
Collaborator
srittau commented Jun 19, 2020

I believe the problem boils down to AnyStr not being constrained by safe's type if safe is not given. I think we need an overload for this case that fixes AnyStr to str.

@JelleZijlstra
Copy link
Member

The relevant types in typeshed are:

_Str = Union[bytes, str]

@overload
def quote(string: str, safe: _Str = ..., encoding: Optional[str] = ..., errors: Optional[str] = ...) -> str: ...
@overload
def quote(string: bytes, safe: _Str = ...) -> str: ...

def urlencode(
    query: Union[Mapping[Any, Any], Mapping[Any, Sequence[Any]], Sequence[Tuple[Any, Any]], Sequence[Tuple[Any, Sequence[Any]]]],
    doseq: bool = ...,
    safe: AnyStr = ...,
    encoding: str = ...,
    errors: str = ...,
    quote_via: Callable[[str, AnyStr, str, str], str] = ...,
) -> str: ...

So mypy is picking the first overload for quote_via (because it's the only one that takes four args), and then errors out because the type of the second arg is the Union and then it tries to set the typevar to the Union. But things should work if we set the typevar to either bytes or str, because function arguments are contravariant.

Maybe there's a way to make this work with overloads in typeshed, but this does seem to be a mypy bug.

@andersk
Copy link
Contributor
andersk commented Jun 23, 2020 •
edited
Loading

The quote_via: Callable[[str, AnyStr, str, str], str] annotation implies that either

  1. only str is used for the entire urlencode call, or
  2. only bytes is used for the entire urlencode call,

but this is not the case: urlencode({'k': b'v'}, quote_via=quote) calls both quote('k', '', None, None) and quote(b'v', '') at runtime.

I think this can be resolved by defining a protocol:

class Quote(Protocol):
    @overload
    def __call__(self, string: str, safe: _Str, encoding: str, errors: str): ...
    @overload
    def __call__(self, string: bytes, safe: _Str): ...

def urlencode(
    query: Union[Mapping[Any, Any], Mapping[Any, Sequence[Any]], Sequence[Tuple[Any, Any]], Sequence[Tuple[Any, Sequence[Any]]]],
    doseq: bool = ...,
    safe: _Str = ...,
    encoding: str = ...,
    errors: str = ...,
    quote_via: Quote = ...,
) -> str: ...

@JelleZijlstra
Copy link
Member

This is fixed on latest mypy + typeshed, likely because of #6345.

@andersk
Copy link
Contributor
andersk commented Dec 22, 2021

#6345 doesn’t resolve the four arguments vs. two arguments issue (as noted by a comment there). The protocol I proposed is still needed.

@JelleZijlstra JelleZijlstra reopened this Dec 22, 2021
@srittau srittau added the stubs: false positive Type checkers report false errors label Dec 22, 2021
@Avasam
Copy link
Collaborator
Avasam commented Apr 10, 2025 •
edited
Loading

As of #9629 urllib.urlencode uses an overload to match the quote_via paramether's type to the safe parameter.

OP's reproduction still passes in latest (1.15) mypy and Python 3.9+

I think that also solved the issue mentioned by @andersk ? If so, this issue can be closed.

srittau added a commit to srittau/typeshed that referenced this issue Apr 11, 2025
@srittau
Simplify and fix urllib.parse.urlencode()
52c4bb4 
Remove overloads and type vars. Introduce a protocol for the
`quote_via` argument. This means that the interface accepted by the
supplied `quote_via` is stricter, and is not dependent on the actual
supplied types in the `query` argument, but must work with all
possible query types.

Closes: python#4234
@srittau srittau mentioned this issue Apr 11, 2025
Merged
@srittau
Copy link
Collaborator
srittau commented Apr 11, 2025 •
edited
Loading

#13815 fixes the problem mentioned by @andersk. It also removes the previously introduces overloads and just requires the quote_via interface to accept more types. If someone wants to write their own quote function, I think it's reasonable to accept the same argument types as the supplied quote and quote_plus functions do (which already comply to that interface), but let's see what mypy primer says.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Simplify and fix urllib.parse.urlencode() srittau/typeshed
6 participants
@andersk @srittau @JelleZijlstra @nipunn1313 @Avasam @hauntsaninja
0