[style] Improve integer completion percentage calculation for accurate representation (358/359 is NOT 100%)

[mitrandir77]

By default rust float formatting doesn't round down - in fact it uses round_ties_even under the hood.

For percentage calculations this is not great - in particular we shouldn't show 100% when the operation is not fully complete.

[djc]

I don't find .floor() an improvement over the current behavior -- now 1/359 is going to be 0% which seems about equally bad. Suggest you use percent_precise instead or implement ProgressTracker to implement custom behavior.

[mitrandir77]

@djc

now 1/359 is going to be 0% which seems about equally bad

I don't think it's equally bad: 1/359 means that the work is not 1% complete yet. 0% seems correct.

I believe rounding down consistently is an improvement and de-facto industry standard. Almost every progress reporting logic I've seen before was using integer division for computing percentage - which made rounding down the default behaviour.

[djc]

I'm still not convinced. For all past indicatif users, this hasn't been enough of a problem to remark upon. IMO conceptually rounding is the least surprising behavior.

[joshkehn]

I'm still not convinced. For all past indicatif users, this hasn't been enough of a problem to remark upon.

Is this proposal logically incorrect though?

I've never seen a progress indicator say 100% complete when there is work still on the table. Showing "100% (358/359)" is a bad UX bug at best and will more likely waste lots of time debugging.

[edward-shen]

As an alternative solution, you can use percent_precise as a format key if you would rather round to 3 decimal places instead.

[mitrandir77]

@edward-shen percent_precise exhibits the same confusing behaviour: given enough items it will start showing not fully complete operations as 100%. (It's how rust fmt formats floats by rounding to nearest number. I'll prep unit test showcasing that.)

Showing 100% on incomplete progress is confusing and bad ux.

[jaheba]

Just to chime in: I don't have very strong opinions on this.

As a user, my main concerns is whether a task is running or has finished. While a task is running, the progress bar provides me with some indication to when the task will be finished. Following that logic, I see the argument to not display "100%" when the task has in fact not finished.

Interestingly, tqdm also exibits the same behaviour and appears to use .round() and not .floor():

from tqdm import tqdm

pbar = tqdm(total=359)
pbar.update(358)

emits:

100%|███████████████████████████████████████████████████▊| 358/359 [00:00<00:00, 167622.33it/s]

I guess this rarely matters in practise, since there need to be more than 200 steps and the underlying task must be slow enough that users can actually observe the described behaviour.

I'm fine with merging this PR, since I don't see any real downside to it. I guess we could even remove these tests, to not "guarantee" any behaviour.

[chris-laplante]

I don't find .floor() an improvement over the current behavior -- now 1/359 is going to be 0% which seems about equally bad. Suggest you use percent_precise instead or implement ProgressTracker to implement custom behavior.

While I don't have a strong opinion, I think I'd prefer using floor instead of round. 1/359 being 0% seems slightly preferable than 358/359 being 100%. But I'd also probably rather not change any existing behavior. What if we added percent_floor and percent_precise_floor? It's yet more format keys, but the case of floor vs round seems to not have a consensus.

(aside: I also wonder if I should just create an indicatif-zoo or indicatif-extras crate or something that includes things we don't want to support in the main crate, but that are oft requested?)

[djc]

(aside: I also wonder if I should just create an indicatif-zoo or indicatif-extras crate or something that includes things we don't want to support in the main crate, but that are oft requested?)

Suggest first making a list in an issue so we can review the evidence of how often different things come up.

[chris-laplante]

(aside: I also wonder if I should just create an indicatif-zoo or indicatif-extras crate or something that includes things we don't want to support in the main crate, but that are oft requested?)

Suggest first making a list in an issue so we can review the evidence of how often different things come up.

Absolutely, will do. I have plenty of things to do before I go creating another crate (like my neglected work on MultiProgress) as well :)

[djc]

What if we added percent_floor and percent_precise_floor? It's yet more format keys, but the case of floor vs round seems to not have a consensus.

I could live with that if you think it's valuable.