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

Tqdm tk #1006

Merged
merged 25 commits into from Jan 10, 2021
Merged

Tqdm tk #1006

merged 25 commits into from Jan 10, 2021

Conversation

richardsheridan
Copy link
Contributor

@richardsheridan richardsheridan commented Jul 15, 2020

This PR adds a Tkinter based GUI for tqdm. It does not attempt to provide the graphs of the current tqdm_gui, but it is more performant, and suitable for use on the command line, within fully-fledged Tkinter apps, or even crazier cases, like the trio guest mode async await tkinter app that I originally built it for. One outstanding bug is that Tkinter does not play well with the Tmonitor functionality, so I disabled it by default for now by setting the class attribute monitor_interval=0.

This PR does not touch the broader, non-experimental functionality of tqdm.

I would boldly suggest making this class the default tqdm.gui.tqdm, as it is lighter-weight and has fewer dependencies compared to the matplotlib.pyplot-based implementation. That could be renamed to tqdm_mpl or something like that, depending on how committed to backwards compatibility you are for these experimental bits.

@richardsheridan richardsheridan mentioned this pull request Jul 15, 2020
9 tasks
@casperdcl casperdcl self-assigned this Jul 15, 2020
@casperdcl casperdcl added p4-enhancement-future 🧨 On the back burner submodule ⊂ Periphery/subclasses to-review 🔍 Awaiting final confirmation labels Jul 15, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 15, 2020

Codecov Report

Merging #1006 into devel will increase coverage by 1.51%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##            devel    #1006      +/-   ##
==========================================
+ Coverage   85.37%   86.89%   +1.51%     
==========================================
  Files          22       22              
  Lines        1279     1335      +56     
  Branches      218      225       +7     
==========================================
+ Hits         1092     1160      +68     
+ Misses        166      151      -15     
- Partials       21       24       +3     

@casperdcl casperdcl changed the base branch from master to devel July 20, 2020 16:07
tqdm/gui.py Outdated Show resolved Hide resolved
Copy link
Sponsor Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good, @richardsheridan - let me know when you think it's ready. Does look like a good idea to recommend it over the mpl implementation.

@richardsheridan
Copy link
Contributor Author

Thanks @casperdcl! I am happy with it right now, so please review the new commits. also, if you have any ideas/suggestions on aesthetic improvements, I'm willing to try to accommodate.

@casperdcl
Copy link
Sponsor Member

casperdcl commented Jul 20, 2020

improvements will probably best be done iteratively; I'm sure planet Earth will chime in.

Can't guarantee when I'll have time to review (I'm not funded for tqdm :P) but hopefully soon.

@codecov-io
Copy link

codecov-io commented Oct 24, 2020

Codecov Report

Merging #1006 into devel will decrease coverage by 0.07%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##            devel    #1006      +/-   ##
==========================================
- Coverage   86.46%   86.39%   -0.08%     
==========================================
  Files          22       22              
  Lines        1456     1463       +7     
  Branches      247      247              
==========================================
+ Hits         1259     1264       +5     
- Misses        165      168       +3     
+ Partials       32       31       -1     

@casperdcl casperdcl added this to In Progress in Casper Oct 25, 2020
@casperdcl casperdcl added this to the Non-breaking milestone Oct 25, 2020
@casperdcl casperdcl moved this from Done to Next Release in Casper Jan 9, 2021
@casperdcl casperdcl added p3-enhancement 🔥 Much new such feature need-feedback 📢 We need your response (question) to-merge ↰ Imminent and removed p4-enhancement-future 🧨 On the back burner labels Jan 9, 2021
Copy link
Sponsor Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @richardsheridan 🎊 - I've finally reviewed this and also rebased onto the current release.

@richardsheridan
Copy link
Contributor Author

Regarding the WIP comments, I think the right way to do those color changes is with ttk.Style, which might lead nicely into changing "states" rather than colours, but I will have to look into it.

I subclassed refresh to make sure we never interact with the lock without changing the interface, as it is unnecessary for the display. I'm happy to drop it if you think that that is better!

@casperdcl
Copy link
Sponsor Member

casperdcl commented Jan 10, 2021

I think the right way to do those color changes is with ttk.Style, which might lead nicely into changing "states" rather than colours, but I will have to look into it.

yes, that would be great.

I subclassed refresh to make sure we never interact with the lock without changing the interface, as it is unnecessary for the display. I'm happy to drop it if you think that that is better!

Probably better to not bother overriding. If the tk updates take a long time to process, the default logic (which supports e.g. locks with timeout) may well speed things up.

@richardsheridan
Copy link
Contributor Author

Changing the progress bar color requires using a non-default theme that may make the bar look less native on some platforms. Also, did you want the text to change color too?

@casperdcl
Copy link
Sponsor Member

casperdcl commented Jan 10, 2021

ideally only the bar colour, not text (as with tqdm.notebook and the standard terminal tqdm.tqdm/tqdm.cli).

Could expose theme/style as an __init__ kwarg... However if it's too difficult, completely ignoring the colour option is also fine.

@richardsheridan
Copy link
Contributor Author

Using the non-default theme has the knock-on effect of changing the style of an entire tk app if the progress bar is embedded in a larger program. It would be the equivalent of setting an entire jupyter notebook to dark mode just to change the colorbar color, so I think skipping the color customization makes sense.

A very interested user can manipulate the _tk_pbar object directly

tqdm/tk.py Outdated Show resolved Hide resolved
Copy link
Sponsor Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-feedback 📢 We need your response (question) p3-enhancement 🔥 Much new such feature submodule ⊂ Periphery/subclasses to-merge ↰ Imminent to-review 🔍 Awaiting final confirmation
Projects
Casper
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants