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

ENH(UX): set "smoothing=0" for any progress bar with "total" in its label #5438

Merged
merged 1 commit into from Feb 25, 2021

Conversation

yarikoptic
Copy link
Member

Original odd experience was reported in #5421 (comment) and it was
due to interplay of two factors:

1 - Total is updated only after git-annex verifies the checksum.

Total progress bar is updated only upon the final "ok" from git-annex
that we got the file. That also involves validation of the checksum by
git-annex. So, even though file is finished downloading, and its progress bar
disappeared, Total would not be updated until that file is "ok"ed by annex.
That takes time, so perceptually to the user (me) it was odd that Total shows
only some little total although a good number of files already "downloaded"

Good news: since Joey implemented "on the fly" checksumming (at least for
built in "special remotes") -- with e.g. 8.20210223+git1-g33af7d193-1~ndall+1 (*)
experience is much better since there is no delay from progressbar disappearing
and the final message arriving and thus Total gets updated.

2 - Default tqdm setting is not good for "rare" updates

See smoothing at https://github.com/tqdm/tqdm . Default 0.3 is a
compromise between providing user with smth between "momentary" and
"mean" speed. When Total is updated only once a minute, that leads to
"current" speed being indeed quite low if most of the time there were no
progress at all ;)

Since "smoothing" is ATM quit specific to tqdm and we have quite a number of
subclasses from ProgressBarBase I decided to just go for an ad-hoc solution
here and make any progress bar which has "total" in its label to use
smoothing=0, i.e. to report the mean.

Altogether with a new git-annex and this total smoothing of 0, Total looks
quite decent now ;)

Closes #5434

*: reminder - installation of a bleeding edge build of git-annex on debians is as difficult as datalad-installer git-annex -m datalad/git-annex now (you would need hub.oauthtoken though in your git config)

…abel

Original odd experience was reported in
datalad#5421 (comment) and it was
due to interplay of two factors:

1 - Total is updated only after git-annex verifies the checksum.

Total progress bar is updated only upon the final "ok" from git-annex
that we got the file.  That also involves validation of the checksum by
git-annex. So, even though file is finished downloading, and its progress bar
disappeared, Total would not be updated until that file is "ok"ed by annex.
That takes time, so perceptually to the user  (me) it was odd that Total shows
only some little total although a good number of files already "downloaded"

Good news: since Joey implemented "on the fly" checksumming (at least for
built in "special remotes") -- with e.g. `8.20210223+git1-g33af7d193-1~ndall+1`
experience is much better since there is no delay from progressbar disappearing
and the final message arriving and thus Total gets updated.

2 - Default tqdm setting is not good for "rare" updates

See `smoothing` at https://github.com/tqdm/tqdm . Default `0.3` is a
compromise between providing user with smth between "momentary" and
"mean" speed.  When Total is updated only once a minute, that leads to
"current" speed being indeed quite low if most of the time there were no
progress at all ;)

Since "smoothing" is ATM quit specific to tqdm and we have quite a number of
subclasses from ProgressBarBase  I decided to just go for an ad-hoc solution
here and make any progress bar which has "total" in its label to use
smoothing=0, i.e. to report the mean.

Altogether with a new git-annex and this total smoothing of 0, Total looks
quite decent now ;)

Closes datalad#5434
@yarikoptic yarikoptic added the UX user experience label Feb 25, 2021
@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #5438 (1a57c04) into maint (8eaf2cf) will decrease coverage by 0.00%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #5438      +/-   ##
==========================================
- Coverage   89.56%   89.56%   -0.01%     
==========================================
  Files         296      296              
  Lines       41756    41758       +2     
==========================================
+ Hits        37398    37399       +1     
- Misses       4358     4359       +1     
Impacted Files Coverage Δ
datalad/ui/progressbars.py 75.42% <50.00%> (-0.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8eaf2cf...1a57c04. Read the comment docs.

Copy link
Contributor

@kyleam kyleam left a comment

Choose a reason for hiding this comment

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

Thank you! Makes sense, and testing a few datalad gets with git-annex 8.20210223, things look better on my end too.

@kyleam kyleam merged commit 2fd098b into datalad:maint Feb 25, 2021
@yarikoptic yarikoptic deleted the enh-pbar-total-ux branch February 25, 2021 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants