Add generic type parameters to MultiDict

[adriangb]

[Kludex]

@adriangb Can you add a description here presenting the case on which this improves developers' lives, and fix the pipeline, please? :pleading_face: :pray:

[adriangb]

Pipeline fixed. MultiDict is a public class so this directly helps anyone who uses it. A simple example of something that didn't work before but now does:

from starlette.datastructures import MultiDict

stuff = MultiDict({"foo": "bar"})
a = stuff.get("foo")
if a is not None:
    print(a.lower())  # used to be Any

[michaeloliverx]

This change would let users remove unnecessary casts when using request.headers.get:

from starlette.requests import Request


def hello_world(request: Request):
    auth = request.headers.get("authorization")
    reveal_type(auth)  # Revealed type is "Any"

[adriangb]

Thank you! Was forgetting where this was used...

[Kludex]

This change would let users remove unnecessary casts when using request.headers.get:

from starlette.requests import Request


def hello_world(request: Request):
    auth = request.headers.get("authorization")
    reveal_type(auth)  # Revealed type is "Any"

This is not correct... #1810 fixed this. You can already see on master:

(starlette3.8.12) ➜  starlette git:(master) ✗ mypy main.py                    
main.py:4: error: Function is missing a return type annotation  [no-untyped-def]
main.py:6: note: Revealed type is "Union[builtins.str, None]"

About this...

Pipeline fixed. MultiDict is a public class so this directly helps anyone who uses it. A simple example of something that didn't work before but now does:

from starlette.datastructures import MultiDict

stuff = MultiDict({"foo": "bar"})
a = stuff.get("foo")
if a is not None:
    print(a.lower())  # used to be Any

I don't think this is a good use case... MultiDict is not even documented.

[adriangb]

I was just programmatically going through our data structures and adding typing to them and since this has been open for months I've lost track of it. So yeah it's possible that this PR doesn't make sense on its own.

It's a shame we made MultiDict public if we only use it internally. Since it is public, I think adding a bit of typing is good, after all we do say 100% type annotated codebase in our README.md.

[adriangb]

In other words, I think this is still valuable but if you disagree please just close this @Kludex, I agree the benefit is not as great as I thought so not worth keeping around if we're not gonna move forward with it

[michaeloliverx]

This change would let users remove unnecessary casts when using request.headers.get:

from starlette.requests import Request


def hello_world(request: Request):
    auth = request.headers.get("authorization")
    reveal_type(auth)  # Revealed type is "Any"

This is not correct... #1810 fixed this. You can already see on master:

(starlette3.8.12) ➜  starlette git:(master) ✗ mypy main.py                    
main.py:4: error: Function is missing a return type annotation  [no-untyped-def]
main.py:6: note: Revealed type is "Union[builtins.str, None]"

Ah I didn't realise MultiDict was different here.

If this doesn't effect user code it might not be worth it in this case, although it is nice to get the correct typing behaviour 😉