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

Type annotate cli.progress_bars #8031

Closed
wants to merge 1 commit into from
Closed

Conversation

McSinyx
Copy link
Contributor

@McSinyx McSinyx commented Apr 13, 2020

This is mainly to set out examples for annotations of args and kwargs to avoid future confusions.

@deveshks, I'd be grateful if you can take a quick look at this before the maintainers starts there reviews, just to sandbox trivial mistakes, if any.

This is mainly to set out examples for annotations of args and kwargs.
@deveshks
Copy link
Contributor

Hi @McSinyx

I also have a PR for this (#7932) but I think it targets some other portions of this file.

Perhaps if you want to, you can tackle the issues I faced as per this comment by @pradyunsg #7932 (review) in this PR as well?

@McSinyx
Copy link
Contributor Author

McSinyx commented Apr 13, 2020

@deveshks, thank you for the heads-up! I'm still looking for the way to solve it. I'll mark this PR is draft for now then.

@McSinyx McSinyx marked this pull request as draft April 13, 2020 09:12
@McSinyx
Copy link
Contributor Author

McSinyx commented Apr 14, 2020

Closing in favor of GH-7932, since the change here is too trivial and the idea has been communicated.

Edit: Based on Python MRO, the mixins should be listed in the following order:

class BaseDownloadProgressBar(DownloadProgressMixin, InterruptibleMixin,
                              WindowsMixin):

As discussed in python/mypy#5837, the current issues with type annotation of mix methods involving elements defined in a non-base class can be solve by moving the classes in here to the front of the inheritance list. However, it is not possible without heavy refactoring, thus I suggest to wait for python/typing#246 to be implemented.

@McSinyx McSinyx closed this Apr 14, 2020
@McSinyx McSinyx deleted the variadic-type branch April 14, 2020 13:32
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants