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

Fix type hint for DjangoObjectTypeOptions.model #1269

Merged
merged 1 commit into from Sep 24, 2022

Conversation

belkka
Copy link
Contributor

@belkka belkka commented Nov 9, 2021

Proper type is typing.Type[Model], not Model

Proper type is `typing.Type[Model]`, not `Model`
@belkka
Copy link
Contributor Author

belkka commented Nov 9, 2021

Also, according to usage,

if fields and fields != ALL_FIELDS and not isinstance(fields, (list, tuple)):

there could be added type hints for fields and exclude:

class DjangoObjectTypeOptions(ObjectTypeOptions):
    ...
    fields = None  # type: Union[List[str], Tuple[str, ...], Literal['__all__']]
    exclude = None  # type: Union[List[str], Tuple[str, ...]]

This would fix mismatching type hint inherited from ObjectTypeOptions.fields, which is annotated as following in graphene:
fields = None # type: Dict[str, Field]

typing.Literal is new in python3.8, so could use str instead of Literal['__all__']

@firaskafri firaskafri self-requested a review September 22, 2022 17:23
@firaskafri firaskafri closed this Sep 23, 2022
@firaskafri firaskafri reopened this Sep 23, 2022
@firaskafri firaskafri closed this Sep 24, 2022
@firaskafri firaskafri reopened this Sep 24, 2022
@firaskafri firaskafri merged commit 60b3032 into graphql-python:main Sep 24, 2022
superlevure pushed a commit to loft-orbital/graphene-django that referenced this pull request Jul 19, 2023
Proper type is `typing.Type[Model]`, not `Model`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants