Removing type ignore comments from cli package

[deveshks]

First stab at fixing #7764

I have initially removed the ones which can be removed and are more straightforward to remove. There are still some remaining which are slightly more non-trivial which I will try in the next commits

[deveshks]

Hi @pradyunsg

As per my comment here #7932 (comment) justifying my usage of getattr, is there another approach I can take here? Maybe just doing self.index = None ,self.avg=None should be a good option as well.

I'll also tackle some of the remaining # type: ignore comments in another PR

[pradyunsg]

I don't want us to add getattr here because of mypy - in general, if we have to add something that we won't do while writing normal code, like a getattr w/ a static string, we shouldn't do that.

Could you revert those changes? We can revisit them in a separate PR.

[deveshks]

I don't want us to add getattr here because of mypy - in general, if we have to add something that we won't do while writing normal code, like a getattr w/ a static string, we shouldn't do that.

Could you revert those changes? We can revisit them in a separate PR.

No worries, I will revert them. Should I then try to handle other # type: ignore comments in this PR, or else we can merge this PR with whatever changes I have made here, and revisit removing other comments in a separate PR.

[pradyunsg]

we can merge this PR with whatever changes I have made here, and revisit removing other comments in a separate PR.

When in doubt, I have a bias toward smaller PRs. :)

[deveshks]

we can merge this PR with whatever changes I have made here, and revisit removing other comments in a separate PR.

When in doubt, I have a bias toward smaller PRs. :)

Agreed @pradyunsg . I have gone ahead and reverted the changes which removed # type: ignore comments at the expense of getattr. I will think of a better to remove them for such situations.

If rest of the changes look good, please go ahead and approve/merge the PR 😊

[deveshks]

Hi @pradyunsg , @xavfernandez

If the changes look good here, could I get this PR approved/merged 😊

[deveshks]

If there are no more changes needed to this PR, may I get this PR approved?

[gutsytechster]

Does the #type: ignore comment here at line 274 also raises some error?

[deveshks]

Does the #type: ignore comment here at line 274 also raises some error?

Yep, If I remove the type: ignore and do something like

def DownloadProgressProvider(progress_bar, max=None):
    # type: (str, Optional[int]) -> Callable[[Iterable[bytes]] , Iterator[bytes]]
    if max is None or max == 0:
        return BAR_TYPES[progress_bar][1]().iter
    else:
        return BAR_TYPES[progress_bar][0](max=max).iter

I get error: "WindowsMixin" has no attribute "iter" on return BAR_TYPES[progress_bar][1]().iter

There is python/mypy#5868 on why this fails and https://mypy.readthedocs.io/en/stable/more_types.html#mixin-classes which provides some suggestions about how to fix it, which I might take a stab at in subsequent PRs after this one is approved/merged.

[pradyunsg]

We should add warn_unused_ignores = True in our mypy global configuration in a follow up PR.

[deveshks]

Thanks @pradyunsg for the approval and merge 😊