Introduce `flake8-pyi` in `pre-commit` checks #1253

hoefling · 2022-11-13T16:36:42Z

I have made things!

This PR adds pre-commit check using flake8-pyi. Most of the warnings are fixed alongside, warnings that conflict with the current mypy state (e.g. python/mypy#11098, python/mypy#12393 or python/mypy#13437) are disabled with an explicit comment linking to the related mypy issue.

Warnings that were disabled globally:

Y021 - looks like docstrings are used here and there, esp. the one for Field stub is useful
Y024 - switching from namedtuple to NamedTuple can be done in an automated fashion, but fields will be typed with Any. Not sure if this will be of any help
Y043 - IMO renaming the aliases is more harm than good

Related issues

Closes #128

…follow flag Signed-off-by: Oleg Hoefling <[email protected]>

Signed-off-by: Oleg Hoefling <[email protected]>

…t case 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]>

adamchainz

Thank you for working on setting this up.

The PR is way too big to review or merge as-is. I would prefer to split this up to activate one or a two rules at a time.

Also I hope you didn’t manually rewrite for changes like PEP 585, pyupgrade can do those automatically. That’s another tool we could add to pre-commit.

adamchainz · 2022-11-13T17:21:02Z

.pre-commit-config.yaml

      - id: flake8
+        additional_dependencies:
+        - flake8-pyi
+        types: []
+        files: ^.*.pyi?$


This has stopped flake8 from running on the repo’s Python files, so we need two configurations. Also, please pin the version of flake8-pyi so there are no surprise failures when a new version is released.

Suggested change

- id: flake8

additional_dependencies:

- flake8-pyi

types: []

files: ^.*.pyi?$

- id: flake8

- id: flake8

additional_dependencies:

- flake8-pyi==22.10.0

types: []

files: ^.*.pyi?$

@adamchainz the glob includes .py files (note the trailing question mark), so the flake8 hook will run on both .py and .pyi files. I can introduce a second hook if you want tho 😎

Good point, missed that! No need for a second hook, I think, unless flake8-pyi rules will be problematic for plain python files.

sobolevn

This looks great! I will merge this asap, so no merge-conflicts on this side will emerge.

We can later work on any further improvements.

Thank you so much, this is amazing! 🎉 👍 😊

sobolevn · 2022-11-13T17:28:32Z

The PR is way too big to review or merge as-is. I would prefer to split this up to activate one or a two rules at a time.

Sorry, @adamchainz I've missed this comment :(

sobolevn · 2022-11-13T17:30:14Z

I personally like to merge big style changes as-is, because otherwise it is really hard to work with merge conflicts and other PRs.

This becomes a kind of a blocker to the whole process.

hoefling · 2022-11-13T17:43:16Z

@adamchainz I'm not against splitting the PR in separate ones, but even fixing a single rule per PR will end in a huge diff (esp. the PEP 604/PEP 585 stuff). Do you want me to go with enabling rules per modules in flake8 config? This is surely doable, but will take time, as there's nothing I can automate on a short sight, will have to redo the scripts first.

Edit: too late, @sobolevn merged already 😎

intgr · 2022-11-13T17:44:31Z

tests/typecheck/test/test_client.yml

+-   case: client_follow_flag
+    main: |
+      from django.test.client import Client
+      client = Client()


What's up with these test changes? Did you base this on some other branch that isn't master?

@intgr whoops you are right, my bad 🤦 this also closed #1252. Will prepare a PR that reverts the changes shortly

intgr · 2022-11-13T17:50:10Z

Thanks! I think the pipe syntax is so much easier on the eyes than Union[] and Optional[].

intgr · 2022-11-14T09:57:47Z

I notice you simplified float | int to float in many places.

Are we affected by this now? python/typeshed#9130

Since mypy 0.990 type promotions was limited.
This means that complex is not longer promoted to int/float, therefore we should adapt the types to list all possible types

sobolevn · 2022-11-14T10:37:19Z

Yes, we are. float | int and bytes | bytearray | memoryview

intgr · 2022-11-14T11:00:09Z

Should we revert this part of the PR?

Maybe hoeflint can re-run the script on an earlier commit, with this particular change disabled. And then submit a PR with the diffs.

sobolevn · 2022-11-14T11:50:01Z

Yes, we should 👍

hoefling · 2022-11-14T13:15:25Z

Yep, can revert that. IIRC there were only float | int and complex | int cases here and there.

…41 rule Signed-off-by: Oleg Hoefling <[email protected]>

) Signed-off-by: Oleg Hoefling <[email protected]> Signed-off-by: Oleg Hoefling <[email protected]>

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. * configure flake8-pyi plugin in pre-commit * fix Y015 * fix Y037 * use PEP 585 container types where possible, replace obsolete typing types with collections.abc * disable Y041 rule * add typealias annotation where feasible, add self annotations * fix failing tests, silence remaining warnings since unclear how to proceed * address review * amend tests * amend rebase error Signed-off-by: Oleg Hoefling <[email protected]>

intgr · 2022-12-08T17:19:15Z

django-stubs/utils/functional.pyi

+    fget: Callable[[Self], _Get] | None
+    def __init__(self: Self, method: Callable[[Self], _Get] | None = ...) -> None: ...
+    def __get__(self: Self, instance: Self | None, cls: type[Self] = ...) -> _Get: ...
+    def getter(self: Self, method: Callable[[Self], _Get]) -> classproperty[_Get]: ...


These self: Self annotations seem to cause regression #1285, I have submitted a PR to fix it in #1287.

Removed unnecessary `Self` hints on classproperty methods. The removal of `self: Self` fixes #1285. Regressed in #1253.

typeddjango#1253 switched this project to use the union type syntax ( https://peps.python.org/pep-0604/ ), which is only available in Python >=3.10. Earlier versions of Python have no support for this syntax.

hoefling added 11 commits November 12, 2022 16:38

only allow redirect_chain attribute in response when client sets the …

5d9fc3d

…follow flag Signed-off-by: Oleg Hoefling <[email protected]>

compat for py3.7

bde7358

Signed-off-by: Oleg Hoefling <[email protected]>

compat for py3.7, fixed

44b7999

Signed-off-by: Oleg Hoefling <[email protected]>

address review comments, extract follow flag checks in a separate tes…

1efa5da

…t case Signed-off-by: Oleg Hoefling <[email protected]>

configure flake8-pyi plugin in pre-commit

d5157ac

Signed-off-by: Oleg Hoefling <[email protected]>

use pep 604 union types where possible

cc78575

Signed-off-by: Oleg Hoefling <[email protected]>

fix Y015

f06c8d0

Signed-off-by: Oleg Hoefling <[email protected]>

use PEP 585 container types where possible, replace obsolete typing t…

cf56059

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

allow isort to reorder non-top imports

add6b4c

Signed-off-by: Oleg Hoefling <[email protected]>

finish PEP 585 changes, use PEP 613 where possible

ba6c37d

Signed-off-by: Oleg Hoefling <[email protected]>

fix remaining flake8-pyi warnings, silence warnings in question

34bbd9d

Signed-off-by: Oleg Hoefling <[email protected]>

hoefling marked this pull request as draft November 13, 2022 16:36

hoefling changed the title ~~Pre commit/flake8 pyi~~ Introduce flake8-pyi in pre-commit checks Nov 13, 2022

fix Y023

4ae7cf9

Signed-off-by: Oleg Hoefling <[email protected]>

hoefling marked this pull request as ready for review November 13, 2022 16:52

fix Y029

6483d4a

Signed-off-by: Oleg Hoefling <[email protected]>

hoefling mentioned this pull request Nov 13, 2022

Setup flake8-pyi to lint stubs #128

Closed

adamchainz reviewed Nov 13, 2022

View reviewed changes

sobolevn approved these changes Nov 13, 2022

View reviewed changes

sobolevn merged commit b81b1bf into typeddjango:master Nov 13, 2022

intgr reviewed Nov 13, 2022

View reviewed changes

hoefling deleted the pre-commit/flake8-pyi branch November 13, 2022 17:46

hoefling mentioned this pull request Nov 13, 2022

Pin flake8-pyi to 22.10.0 #1254

Merged

hoefling mentioned this pull request Nov 13, 2022

revert changes in #1252 leaked into master #1255

Merged

hoefling added a commit to hoefling/django-stubs that referenced this pull request Nov 14, 2022

revert changes done in typeddjango#1253 that satisfy Y041, disable Y0…

cd007f7

…41 rule Signed-off-by: Oleg Hoefling <[email protected]>

This was referenced Nov 14, 2022

revert changes done in #1253 that satisfy Y041, disable Y041 rule #1256

Merged

Introduce flake8-pyi in pre-commit checks typeddjango/djangorestframework-stubs#286

Merged

sobolevn pushed a commit that referenced this pull request Nov 15, 2022

revert changes done in #1253 that satisfy Y041, disable Y041 rule (#1256

a1c192c

) Signed-off-by: Oleg Hoefling <[email protected]> Signed-off-by: Oleg Hoefling <[email protected]>

intgr mentioned this pull request Dec 8, 2022

Fix @classproperty decorator #1287

Merged

intgr reviewed Dec 8, 2022

View reviewed changes

intgr added a commit that referenced this pull request Dec 9, 2022

Fix @classproperty decorator (#1287)

16fde01

Removed unnecessary `Self` hints on classproperty methods. The removal of `self: Self` fixes #1285. Regressed in #1253.

intgr mentioned this pull request Jan 4, 2023

Improve some DB expression types #1243

Merged

flaeppe mentioned this pull request Aug 2, 2024

ManyToManyField is not complete #754

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `flake8-pyi` in `pre-commit` checks #1253

Introduce `flake8-pyi` in `pre-commit` checks #1253

hoefling commented Nov 13, 2022 •

edited

Loading

adamchainz left a comment

adamchainz Nov 13, 2022

hoefling Nov 13, 2022

adamchainz Nov 13, 2022

sobolevn left a comment

sobolevn commented Nov 13, 2022

sobolevn commented Nov 13, 2022

hoefling commented Nov 13, 2022 •

edited

Loading

intgr Nov 13, 2022

hoefling Nov 13, 2022

intgr commented Nov 13, 2022

intgr commented Nov 14, 2022

sobolevn commented Nov 14, 2022

intgr commented Nov 14, 2022

sobolevn commented Nov 14, 2022

hoefling commented Nov 14, 2022

intgr Dec 8, 2022

Introduce flake8-pyi in pre-commit checks #1253

Introduce flake8-pyi in pre-commit checks #1253

Conversation

hoefling commented Nov 13, 2022 • edited Loading

I have made things!

Related issues

adamchainz left a comment

Choose a reason for hiding this comment

adamchainz Nov 13, 2022

Choose a reason for hiding this comment

hoefling Nov 13, 2022

Choose a reason for hiding this comment

adamchainz Nov 13, 2022

Choose a reason for hiding this comment

sobolevn left a comment

Choose a reason for hiding this comment

sobolevn commented Nov 13, 2022

sobolevn commented Nov 13, 2022

hoefling commented Nov 13, 2022 • edited Loading

intgr Nov 13, 2022

Choose a reason for hiding this comment

hoefling Nov 13, 2022

Choose a reason for hiding this comment

intgr commented Nov 13, 2022

intgr commented Nov 14, 2022

sobolevn commented Nov 14, 2022

intgr commented Nov 14, 2022

sobolevn commented Nov 14, 2022

hoefling commented Nov 14, 2022

intgr Dec 8, 2022

Choose a reason for hiding this comment

Introduce `flake8-pyi` in `pre-commit` checks #1253

Introduce `flake8-pyi` in `pre-commit` checks #1253

hoefling commented Nov 13, 2022 •

edited

Loading

hoefling commented Nov 13, 2022 •

edited

Loading