Skip to content

Introduce flake8-pyi in pre-commit checks #286

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

Merged
merged 10 commits into from
Nov 16, 2022

Conversation

hoefling
Copy link
Contributor

I have made things!

This is a PR similar to typeddjango/django-stubs#1253, only for rest_framework-stubs.

All statements that are unclear on how to proceed are marked with # noqa: in the code, with the exception of compat.pyi, which has additional rules turned off in setup.cfg.

There are some low-hanging fruits to fix, e.g.

max_whole_digits = Optional[int]

should obviously be

max_whole_digits: int | None

or

default_schema_renderers = None

which could be set to default_schema_renderers: None, or removed altogether.

include_root_view: bool
include_format_suffixes: bool
root_view_name: str
default_schema_renderers = None # noqa: Y026
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think None is the default value and should be removed.

You can type this as : Any for now.

Copy link
Contributor

@intgr intgr Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you/I can fix these in a follow-up just as well. Up to you.

@intgr
Copy link
Contributor

intgr commented Nov 15, 2022

To fix the tests, you need a small change in typecheck_tests.py:

--- a/scripts/typecheck_tests.py
+++ b/scripts/typecheck_tests.py
@@ -239,7 +239,7 @@ IGNORED_ERRORS = {
         'Argument "data" to "ValidationSerializer" has incompatible type "str"',
     ],
     "test_validation_error.py": [
-        'Argument "detail" to "ValidationError" has incompatible type "Tuple[str, str]"; expected "Union[str, List[Any], Dict[str, Any], None]"',  # noqa: E501
+        'Argument "detail" to "ValidationError" has incompatible type "Tuple[str, str]"; expected "Optional[Union[str, List[Any], Dict[str, Any]]]"',  # noqa: E501
     ],
     "test_validators.py": [
         'Argument "queryset" to "BaseUniqueForValidator" has incompatible type "object";'

@intgr
Copy link
Contributor

intgr commented Nov 15, 2022

Sorry, I merged another PR to master and there are merge conflicts now.

I'll defer any other .pyi changes until this is closed.

Copy link
Contributor

@intgr intgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the conflicts are solved and typecheck_tests.py updated, I can merge this.

Signed-off-by: Oleg Hoefling <[email protected]>
Signed-off-by: Oleg Hoefling <[email protected]>
…ypes with collections.abc

Signed-off-by: Oleg Hoefling <[email protected]>
Signed-off-by: Oleg Hoefling <[email protected]>
Signed-off-by: Oleg Hoefling <[email protected]>
Signed-off-by: Oleg Hoefling <[email protected]>
@hoefling hoefling force-pushed the pre-commit/flake8-pyi branch from 9998f47 to c4b0dd3 Compare November 16, 2022 12:14
@intgr intgr self-requested a review November 16, 2022 12:15
@hoefling
Copy link
Contributor Author

@intgr sorry for the delay, could not finish the PR on Monday. And thank you for the hint for fixing the test error!

@hoefling
Copy link
Contributor Author

Will amend the remaining issues shortly

@intgr
Copy link
Contributor

intgr commented Nov 16, 2022

There seems to be a missing import:

djangorestframework-stubs/djangorestframework-stubs/rest_framework-stubs/test.pyi:70: error: Name "Response" is not defined (diff)

Signed-off-by: Oleg Hoefling <[email protected]>
@hoefling
Copy link
Contributor Author

@intgr yep I didn't pay enough attention when rebasing 🤦‍♀️

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Very much appreciated.

Copy link
Contributor

@intgr intgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome.

@intgr intgr merged commit 158fd4f into typeddjango:master Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants