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 default spinner collection #282

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

alicedb2
Copy link

@alicedb2 alicedb2 commented Nov 8, 2023

I like spinners with my ProgressUnknown(), so here's a collection of 110 of them. The first 89 are stolen (with credits) from https://github.com/briandowns/spinner, the rest (90-110) I had fun creating last evening.

Summary of the changes (see below for a demo):

  • Includes a spinnercollection of 110 spinners,
  • Added the possibility of specifying spinners that are AbstractVector{<:AbstractString}. Were there any reason to restrict them to a single Char?
  • next!(prog, spinner=idx) is equivalent to next!(prog, spinner=spinnercollection[idx])
  • There's a fun demospinners() function that shows them all in action, but also
    • demospinners(idx) to look at a specific spinner in the collection,
    • demospinners(string), demospinners(vector_of_chars), and demospinners(vector_of_strings) to look at your own custom spinner,
    • demospinners(vector_of_vectors_of_strings) to look at your own set of custom spinners in a nicely aligned table. demospinners() is just demospinners(spinnercollection).
    • It adds IterTools.jl as an additional dependency because of zip_longest. I wish I could do without it, the alignment algo is very inefficient anyway and should be improved.

I don't know if anyone is interested, but if so then I'm looking for input on how to make the code align better with the organiziation/idioms of ProgressMeter.jl. Maybe we could also shrink the collection a bit and be more selective about which spinners to include.

Demo (the gif doesn't loop perfectly like the spinners in the demo do for obvious reasons, the LCM of all spinner lengths is 27,003,936,960):
spinners

and so you can get things like this
progressunknown

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 35 lines in your changes are missing coverage. Please review.

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

❗ Current head fa0746a differs from pull request most recent head eaaa4a6. Consider uploading reports for the commit eaaa4a6 to get more accurate results

Files Patch % Lines
src/spinners.jl 0.00% 34 Missing ⚠️
src/ProgressMeter.jl 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
- Coverage   96.81%   90.12%   -6.69%     
==========================================
  Files           1        2       +1     
  Lines         533      567      +34     
==========================================
- Hits          516      511       -5     
- Misses         17       56      +39     

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

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.

1 participant