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

Use flake8 type checking #4787

Merged
merged 7 commits into from
Nov 18, 2021

Conversation

radoering
Copy link
Member

Pull Request Check List

Relates-to: #4776

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

Awesome! I like this a lot more than I even thought I would -- it really makes the usage of imports unambiguous.

A few corrections/nits to pick, and then also one question -- since upstream seems open to adding a whitelist, is it waiting so we can drop # noqa? The good news is when the whitelist is added, yesqa should automate removing those comments.

Alternatively, there is https://pypi.org/project/future-annotations/, but I'm not sure how it will interact with the rest of our tooling.

.flake8 Outdated Show resolved Hide resolved
@@ -1 +1 @@
from poetry.masonry.builders.editable import EditableBuilder
from poetry.masonry.builders.editable import EditableBuilder # noqa: TC002
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we should exempt __init__.py from TC002 like we do from F401 in .flake8.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure. Probably, suitable for most __init__.py files. However, some files like poetry/mixology/__init__.py should probably not be excluded. Maybe, the same applies to F401. 🤷‍♂️ I'll change it if you like.

src/poetry/utils/password_manager.py Outdated Show resolved Hide resolved
@radoering
Copy link
Member Author

A few corrections/nits to pick, and then also one question -- since upstream seems open to adding a whitelist, is it worth adding those so we can drop # noqa? The good news is when the whitelist is added, yesqa should automate removing those comments.

I'll try the whitelist as soon as it has been added upstream. That should remove a lot of # noqa.

Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

Beautiful 😍

@finswimmer finswimmer merged commit 9e31f7a into python-poetry:master Nov 18, 2021
edvardm pushed a commit to edvardm/poetry that referenced this pull request Nov 24, 2021
* Use flake8-type-checking

* Fixes for flake8-type-checking violations

* Set enable-extensions before ignores and excludes

* Remove noqa duplicate

* Use new flake8-type-checking version with type-checking-exempt-modules feature for imports of typing and typing-extensions so that noqa comments are not required anymore

* Ignore TC002 in __init__ files

* Remove noqa from pyi files because they are not checked anyway
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants