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

Werkzeug EnvironHeaders is not considered a Mapping #3569

Closed
libre-man opened this issue Jan 3, 2020 · 6 comments
Closed

Werkzeug EnvironHeaders is not considered a Mapping #3569

libre-man opened this issue Jan 3, 2020 · 6 comments

Comments

@libre-man
Copy link
Contributor

It seems that the EnvironHeaders class of werkzeug (and probably the base class Headers too) is not compatible with Mapping, however it looks like it should be compatible (it implements all needed methods, and functions like one).

@srittau
Copy link
Collaborator

srittau commented Jan 3, 2020

Do you have an example that you think should type check but doesn't?

@libre-man
Copy link
Contributor Author

Here is small example:

from flask import request
import typing as t

T = t.TypeVar('T')
Z = t.TypeVar('Z')

def get(kv: t.Mapping[T, Z], key: T) -> Z:
    return kv[key]

get(request.headers, 'a_header')

On which mypy errors with: test.py:10: error: Argument 1 to "get" has incompatible type "EnvironHeaders"; expected "Mapping[str, <nothing>]".

@srittau
Copy link
Collaborator

srittau commented Jan 5, 2020

I opened #3576 for the underlying issue. My best suggestion for now is to use a custom protocol for your own code and - depending on the outcome of the discussion in #3576 - submit PRs for code in typeshed.

@gward
Copy link
Contributor

gward commented Jul 7, 2020

For posterity, here is a custom protocol that works for me in one particular place:

from typing import overload
import typing_extensions

class HeadersType(typing_extensions.Protocol):
    """common subtype of Mapping[str, str] and werkzeug.datastructures.EnvironHeaders"""
    @overload
    def get(self, k: str) -> Optional[str]:
        ...

    @overload
    def get(self, k: str, default: str) -> str:
        ...

This is not a general solution to the problem! But I just spent at least an hour banging my head against this until I got it working. Hopefully this can save someone else some time in the future.

I just had to change my code from

headers: Mapping[str, str]

to

headers: HeadersType

and now mypy is happy.

srittau added a commit to srittau/typeshed that referenced this issue Jul 10, 2020
typing.Mapping is not a protocol, which has caused problems in the past.
(E.g. python#3569, see also python#3576.) This
introduces a few narrow protocols to _typeshed.pyi that can be used in
place of Mapping.

Not all uses of Mapping can be replaced. For example, cgi.FieldStorage
explictly checks whether the supplied headers argument is a Mapping
instance.
@srittau
Copy link
Collaborator

srittau commented Jul 10, 2020

A brief addendum on why we can't just derive Headers from Mapping: Since Mapping is not a protocol, the following code returns False at runtime:

from collections.abc import Mapping
from werkzeug.datastructures import Headers

isinstance(Headers(), Mapping)  # False

Deriving Headers from Mapping would introduce a false negatives. For example, cgi.FieldStorage takes a headers arguments, but checks for Mapping explicitly:

from cgi import FieldStorage
from werkzeug.datastructures import Headers

FieldStorage(headers=Headers())  # throws a TypeError

JelleZijlstra pushed a commit that referenced this issue Jul 12, 2020
typing.Mapping is not a protocol, which has caused problems in the past.
(E.g. #3569, see also #3576.) This
introduces a few narrow protocols to _typeshed.pyi that can be used in
place of Mapping.

Not all uses of Mapping can be replaced. For example, cgi.FieldStorage
explictly checks whether the supplied headers argument is a Mapping
instance.
@srittau
Copy link
Collaborator

srittau commented May 18, 2021

Closing as the werkzeug stubs will be removed in a few months.

@srittau srittau closed this as completed May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants