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

Code Quality: Incremental adoption of mypy type checks #5017

Closed
5 of 7 tasks
finswimmer opened this issue Jan 10, 2022 · 14 comments · Fixed by #5399
Closed
5 of 7 tasks

Code Quality: Incremental adoption of mypy type checks #5017

finswimmer opened this issue Jan 10, 2022 · 14 comments · Fixed by #5399
Labels
kind/enhancement Not a bug or feature, but improves usability or performance

Comments

@finswimmer
Copy link
Member

finswimmer commented Jan 10, 2022

poetry and poetry-core are full type annotated and checking types by mypy is prepared. However these checks are deactivated for most of the modules.

In this ticket I like to invite everybody to pick one or more modules in poetry and poetry-core and fix errors reported by mypy. Please link any PR you are starting to this issue.

In process:

@finswimmer finswimmer added kind/enhancement Not a bug or feature, but improves usability or performance Good First Issue labels Jan 10, 2022
@finswimmer finswimmer changed the title Incremental adoption of mypy typechecks Incremental adoption of mypy type checks Jan 10, 2022
@finswimmer finswimmer changed the title Incremental adoption of mypy type checks Code Quality: Incremental adoption of mypy type checks Jan 10, 2022
@dimbleby
Copy link
Contributor

@finswimmer can I suggest that you'll likely get better engagement with this sort of thing if maintainers are able to merge the PRs reasonably promptly? I know that I would much more likely have felt encouraged to continue the work of python-poetry/poetry-core#253 if I saw it being committed.

@finswimmer
Copy link
Member Author

Hello @dimbleby,

I'm one of the maintainers (just if you haven't know it). I absolutely agree with your sentiment that reacting on a PR in short time would get better engagement. But unfortunately this is not always possible. I think you already know it. We are all doing this in our free time. And sometimes one has to decide whether to review a PR a comment on some of the 30-50 new issues/questions/feature request per week. Some reviews require also discussion between the maintainers, which takes time.

If you feel that something takes to long, please ping us in the isssue/PR or on the discord server.

fin swimmer

@dimbleby
Copy link
Contributor

I intend no upset, I'm here because I think poetry is a good thing, and I'm nothing but grateful to those who've got us this far. Please be assured that I am not trying to knock or discourage anyone, my apologies if it seemed otherwise.

But yeah, my experience is that contributing to poetry generally is a slow business. Probably #4595 would have been a better place to talk about it though.

@abn
Copy link
Member

abn commented Mar 25, 2022

@krakenbite this is already under way. I'd recommend you pick a module or test ignored (https://github.com/python-poetry/poetry/blob/master/pyproject.toml#L113) and fix it. Don't try to do it all in a single PR that's hard to review. :)

@tarun-jethwani
Copy link
Contributor

tarun-jethwani commented Mar 27, 2022

Hi @abn @python-poetry/triage @finswimmer, I would also like to work on this issue, infact started working on it, I am a first time contributor to poetry, have already read Contributing.md guidelines and familiar with necessary practices, I would like to submit PR one per module out of all the approx. 25 modules listed under [[tool.mypy.overrides]] , for starting I would like to pick up this "poetry.console.application" module,

I will also try to submit PR with only closely related changes and will not try not to do too much at once in single PR, I will first try to submit a Draft PR first

@tarun-jethwani
Copy link
Contributor

tarun-jethwani commented Mar 31, 2022

Hi @abn, @finswimmer @branchvincent , I will take up poetry.mixology to solve for mypy errors, as there is already an open PR for poetry.console

@anthonymichaelclark
Copy link
Contributor

Link to PR for password_manager

@tarun-jethwani
Copy link
Contributor

tarun-jethwani commented Apr 3, 2022

@branchvincent please review my PR [https://github.com//pull/5402] - fix for mypy errors in poetry.mixology package

@tarun-jethwani
Copy link
Contributor

Hi @abn , @finswimmer , @branchvincent @dimbleby , Please approve my PR https://github.com/python-poetry/poetry/pull/5402 pending since yesterday, I have completed all the required changes

@tarun-jethwani
Copy link
Contributor

Hi @abn , @finswimmer , @branchvincent , I have resolved merge conflicts in this PR fix mypy errors for poetry.mixology , please review this PR

@dimbleby
Copy link
Contributor

dimbleby commented Apr 26, 2022

open typechecking MRs (list may not be complete) include

Aside from the minor detail that these aren't yet merged, we're very nearly done!

@Secrus
Copy link
Member

Secrus commented May 21, 2022

As of today, only poetry.utils.env is left to be properly typed. Good job everyone!

This was referenced May 21, 2022
@Secrus
Copy link
Member

Secrus commented May 23, 2022

This is now completed.

@Secrus Secrus closed this as completed May 23, 2022
Copy link

github-actions bot commented Mar 2, 2024

This issue 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 Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement Not a bug or feature, but improves usability or performance
Projects
None yet
7 participants