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

Drop # type: ignore comments in cli subpackage #7764

Closed
pradyunsg opened this issue Feb 21, 2020 · 7 comments
Closed

Drop # type: ignore comments in cli subpackage #7764

pradyunsg opened this issue Feb 21, 2020 · 7 comments
Labels
state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes

Comments

@pradyunsg
Copy link
Member

Follow up on #6727 (review)

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Feb 21, 2020
@pradyunsg pradyunsg added state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes labels Feb 21, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Feb 21, 2020
@mathagician
Copy link

Hello @pradyunsg, I would like to work on this issue. Could you please assign this to me?

@pradyunsg
Copy link
Member Author

@mathagician Sure! Go ahead! :)

Note that this issue will entail fixing the mypy issues, that are being silenced by the ignore comments.

@rohitsanj
Copy link

Hey @mathagician let me know if you are still working on this, I'd like to work on it as well 😄

@pradyunsg I'm not sure how I can reproduce the mypy issues in cli subpackage. I ran the command tox -e lint but it just showed some mypy errors in another file and failed with this error:

ERROR: InvocationError for command /Users/rohitsanjay/pip/.tox/lint/bin/pre-commit run --all-files --show-diff-on-failure (exited with code 1)

@pradyunsg
Copy link
Member Author

@rohitsanj please post the entire output, otherwise it is not possible to get the required context to help answer your questions.

@deveshks
Copy link
Contributor

Hi @pradyunsg

Can I take up this PR? Also what is the fix you are looking for the PR here? Is the aim to remove all #type: ignore comments and fix the lines of code where it was present one by one?

@pradyunsg
Copy link
Member Author

Yea, basically. :)

@deveshks
Copy link
Contributor

deveshks commented Jun 7, 2020

I was experimenting a bit, and found that if InterruptibleMixin
and WindowsMixin are defined as subclasses of pip._vendor.progress.Infinite and
DownloadProgressMixin as a subclass of [pip._vendor.progress.Progress]](https://github.com/pypa/pip/blob/master/src/pip/_vendor/progress/__init__.py#L136), most of the remaining type: ignore comments can be gotten rid of.

But I am not sure this is the correct way, and if it defeats the purpose of defining these classes as Mixin.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

No branches or pull requests

4 participants