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

add parameter for fractional bar length #285

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

blwh
Copy link

@blwh blwh commented Nov 14, 2023

I find that the progress bar usually overflows when the ETA quickly converges from a large to a small value. Because of this, I want to make the bar shorter. However, since the TTY width is not immediately obvious to me, I would rather like to scale the length of the bar by some fraction. This PR adds this feature.

Summary of changes:

  • Added the parameter barlen_fraction to the Progress-struct, which can take on any float. This means that the bar can be scaled negatively and larger than 1.

I might have added too much to the testing, and the argument might be too verbose.

src/deprecated.jl Outdated Show resolved Hide resolved
@MarcMush
Copy link
Collaborator

for tests, add a few tests with the new feature but don't change old ones. That way we can check that the change is not breaking. All old tests should still pass (including the deprecated ones, which is not the case at the moment)

@blwh blwh force-pushed the master branch 3 times, most recently from 49caa8b to 237e252 Compare November 21, 2023 22:28
@blwh
Copy link
Author

blwh commented Nov 22, 2023

I've added tests. However, the performance test in core.jl sometimes fails. It seems to be due to the required truncation on line 209

    full_width = trunc(Int, (displaysize(output)[2]*width_fraction))

It's not consistent, as it sometimes (mostly) fails on my machine. I ran the performance test prog_perf(n) on line 28 in core.jl and got on average that the code with the truncation is about 7% slower (0.144s vs 0.135s), which makes the test on line 51 in core.jl barely fail

    @test @elapsed(prog_perf(10^7)) < 9*@elapsed(noprog_perf(10^7))

I think the 9 is a bit arbitrary and that the potential slowdown of the truncation is not bad enough. If not, I do not have any other suggestions to improve the code.

@blwh blwh requested a review from MarcMush November 22, 2023 18:50
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5b3bd1d) 96.81% compared to head (f854e4c) 96.81%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #285   +/-   ##
=======================================
  Coverage   96.81%   96.81%           
=======================================
  Files           1        1           
  Lines         533      534    +1     
=======================================
+ Hits          516      517    +1     
  Misses         17       17           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MarcMush
Copy link
Collaborator

MarcMush commented Jan 28, 2024

do you have a reproducible example of your original problem (the progressbar overflowing)?
I believe it might be a problem with #233 where a space is added without reducing the size of the progressbar

edit: nvm I could reproduce it with testfunc4() in the tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants