graphql-python / graphql-core

A Python 3.6+ port of the GraphQL.js reference implementation of GraphQL.
MIT License
512 stars 136 forks source link

Allow user to pass in a custom resolve info context type (#213) #214

Closed fedirz closed 8 months ago

fedirz commented 8 months ago

Usage example:

from typing import Generic, NamedTuple, TypeVar

TContext = TypeVar("TContext")

class GraphQLResolveInfo(Generic[TContext], NamedTuple):
    ...
    context: TContext
    ...

GraphQLResolveInfo(None) # Ok
GraphQLResolveInfo({"asdf": 1234}) # Ok

type Context = dict[str, int]

UserGraphQLResolveInfo = GraphQLResolveInfo[Context]

UserGraphQLResolveInfo(None)
 # │     Argument of type "None" cannot be assigned to parameter "context" of type "Context" in function "__init__" Pyright (reportArgumentType) [17, 24]
 # │        "None" is incompatible with "Context"

UserGraphQLResolveInfo({"asdf": 1234}) # Ok
Cito commented 8 months ago

How did you actually produce that type error? I tried with mypy 1.8, but couldn't reproduce it.

Update: I can reproduce it now, but not if I use a type alias. I need to specify the type directly. Seems you used Pylance, right?

fedirz commented 8 months ago

How did you actually produce that type error? I tried with mypy 1.8, but couldn't reproduce it.

Update: I can reproduce it now, but not if I use a type alias. I need to specify the type directly. Seems you used Pylance, right?

No, the error came from pyright

graphql-core on  generic-resolve-info-context [!?] is 📦 v3.3.0a4
❄ ❯ python --version
Python 3.12.2

graphql-core on  generic-resolve-info-context [!?] is 📦 v3.3.0a4
❄ ❯ pyright --version
pyright 1.1.349

graphql-core on  generic-resolve-info-context [!?] is 📦 v3.3.0a4
❄ ❯ pyright src/graphql/test.py
/home/ted/code/graphql-core-dev/graphql-core/src/graphql/test.py
  /home/ted/code/graphql-core-dev/graphql-core/src/graphql/test.py:17:24 - error: Argument of type "None" cannot be assigned to parameter "context" of type "Context" in function "__init__"
    "None" is incompatible with "Context" (reportArgumentType)
1 error, 0 warnings, 0 informations
fedirz commented 8 months ago

Can you also add a test that serves as a usage example, like above in the description.

You can simulate a type error with a "type: ignore" comment. It should say "unnecessary type ignore" if there is no type error.

Is there already a test in the codebase that does something similar, which I can use as an example?

Cito commented 8 months ago

How did you actually produce that type error? I tried with mypy 1.8, but couldn't reproduce it. Update: I can reproduce it now, but not if I use a type alias. I need to specify the type directly. Seems you used Pylance, right?

No, the error came from pyright

Makes sense since Pylance is based on Pyright.

Cito commented 8 months ago

The idea is to add a test that serves as a usage example and where running the test or type checking of the test code would fail if you revert the change in this PR.

Instead of a comment like "# this gives an error" you would simply write "# type: ignore". This comment tells type checkers like pyright or mypy to ignore the typing error, so that the type checking that is also run on the test code would not fail. On the other hand, if there is not the expected type error, mypy is configured to report a superfluous "type: ignore" comment. You can test everything with tox and the included tox.ini file.

But you can also leave the tests to me, no problem.

fedirz commented 8 months ago

The idea is to add a test that serves as a usage example and where running the test or type checking of the test code would fail if you revert the change in this PR.

Instead of a comment like "# this gives an error" you would simply write "# type: ignore". This comment tells type checkers like pyright or mypy to ignore the typing error, so that the type checking that is also run on the test code would not fail. On the other hand, if there is not the expected type error, mypy is configured to report a superfluous "type: ignore" comment. You can test everything with tox and the included tox.ini file.

But you can also leave the tests to me, no problem.

Yeah, if you could add the test, that'd be great. Thanks for the help on this!