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

Make the ProgressBar class API #399

Closed
pombredanne opened this issue Aug 6, 2015 · 10 comments
Closed

Make the ProgressBar class API #399

pombredanne opened this issue Aug 6, 2015 · 10 comments

Comments

@pombredanne
Copy link
Contributor

@mitsuhiko The click._termui_impl.ProgressBar is a wonderful and magical piece of code.

There are cases though where subclassing this can be needed such as providing a few defaults progress messages such as on enter or exit of the context manager for intro and summaries.

This is especially useful to avoid otherwise naughty code mixing progress reporting and actual processing

Nothing prohibits this of course, but if it were made API, subclassing would benefit from a tad more stability with new upgrades. The overall interface of the class has been rather stable for about one year now with only a minor change to support manually updating more than one step at a time....

So IMHO this would benefit to be exposed in a non _ underscore module.

@pombredanne
Copy link
Contributor Author

BTW the code I am using this in is here https://github.com/nexB/scancode-toolkit/blob/e3b57934a0d6e41a7e3ee6e8261912d3b91764cb/src/scancode/utils.py#L64

It is rather nasty and tangled and under heavy refactoring to be cleaner and less spaghetti like ... ;)

@flying-sheep
Copy link
Contributor

i’m 👍ing this!

i just made the progressbar abstraction smart-progress, which i at first wanted to base on tqdm, but which now relies on that class.

sadly that means relying on an undocumented, private API.

from my usage you can maybe derive what a more general interface would need. IMHO:

  • it’s already pretty awesome to extend. apart from ignoring a few parameters/fields, i basically just have to override __enter__, render_finish, and render_progress.
  • I’d suggest to create a render_start method and call that instead of render_progress in __enter__.
  • also please extract this and this part into a info_bits or format_info_bits method/property which would allow to easily use the information in without copying code from format_progress_line

thank you very much!

@flying-sheep
Copy link
Contributor

@pombredanne your link is broken. protip® from github: press y to get a canonical link to a commit that will never break by changing master

@untitaker
Copy link
Contributor

I recall Armin mentioning one time that he is fundamentally unhappy with the ProgressBar API.

@flying-sheep
Copy link
Contributor

well, it’s still easily extensible and its functionality is too useful not to use.

and being able to either use iteration or the update method is also very useful.

@pombredanne
Copy link
Contributor Author

@flying-sheep link updated
You can see there an enhanced progress bar, a more verbose progress logger and a noop progress bar (displaying nothing). The three are then used to display more or less based on command options.

https://github.com/nexB/scancode-toolkit/blob/e3b57934a0d6e41a7e3ee6e8261912d3b91764cb/src/scancode/utils.py#L64

@flying-sheep
Copy link
Contributor

also useful would be feature detection if the shell can do unicode and then go to a finer-grained bar like this.

that would also benefit from a more flexible format_progress_line. e.g. by adding a format_bar method that can be overridden

@mitsuhiko
Copy link
Contributor

I think it would be nice if people could experiment around with it somewhere else and when we found a better way, we redo it in click. I'm not super happy with how it works.

@flying-sheep
Copy link
Contributor

could you voice your problems with it?

@davidism
Copy link
Member

ProgressBar's implementation is pretty messy. It works, but there are better solutions out there, such as tqdm. For now I'd rather not elevate its API more than is already documented.

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

No branches or pull requests

6 participants