Skip to content

Change attributes of RichProgressBarTheme dataclass #10454

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

Merged
merged 9 commits into from
Nov 11, 2021

Conversation

Raahul-Singh
Copy link
Contributor

@Raahul-Singh Raahul-Singh commented Nov 10, 2021

Just some name changes to Rich Progress Bar Themes.

Closes #10435

To visually test the params, I created the following theme:

            theme=RichProgressBarTheme(
                text_color="bright_red",
                progress_bar_complete="bright_green",
                progress_bar_finished="bright_yellow",
                progress_bar_pulse="bright_blue",
                batch_process="bright_magenta",
                time="bright_cyan",
                processing_speed="bright_white",
            )

This is what I get:

Screenshot 2021-11-10 at 2 33 12 PM

Screenshot 2021-11-10 at 2 32 22 PM

Considering this, I propose we change the params to the following:

  • progress_bar_complete -> progress_bar_completed_so_far
  • progress_bar_finished -> progress_bar_on_completion
  • batch_process -> batch_progress

Also, I am a little confused about what progress_bar_pulse does. Could you guys help me out with this?

@kaushikb11
Copy link
Contributor

@Raahul-Singh

I am a little confused about what progress_bar_pulse does.

progress_bar_pulse is used when IterableDataset is being processed.

@Raahul-Singh
Copy link
Contributor Author

@Raahul-Singh

I am a little confused about what progress_bar_pulse does.

progress_bar_pulse is used when IterableDataset is being processed.

Ah, makes sense!
Thanks @kaushikb11 !

@kaushikb11 kaushikb11 changed the title param name updates Change attributes of RichProgressBarTheme dataclass Nov 10, 2021
Copy link
Contributor

@SeanNaren SeanNaren left a comment

Choose a reason for hiding this comment

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

@kaushikb11 with the thorough review :)

This is awesome @Raahul-Singh and I really like the theme you have! We had to go for a more monotone theme primarily out of user cases (something standard rather than bright and colourful).

I personally would be up for themes, but this is just because I like pretty visuals :P

@Raahul-Singh
Copy link
Contributor Author

Haha, @SeanNaren you and me both. I am planning on making a full cyberpunk-esque theme for my projects. Gotta thank the Lightning team for introducing me to rich. I simply love this library!

@Raahul-Singh
Copy link
Contributor Author

Thank you @kaushikb11 for all the help and feedback :)

@mergify mergify bot added the ready PRs ready to be merged label Nov 10, 2021
Copy link
Contributor

@tchaton tchaton left a comment

Choose a reason for hiding this comment

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

LGTM !

Copy link
Contributor

@rohitgr7 rohitgr7 left a comment

Choose a reason for hiding this comment

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

LG!

@rohitgr7 rohitgr7 enabled auto-merge (squash) November 10, 2021 18:43
@codecov
Copy link

codecov bot commented Nov 11, 2021

Codecov Report

Merging #10454 (c571f90) into master (aad8642) will decrease coverage by 4%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #10454    +/-   ##
========================================
- Coverage      92%      89%    -4%     
========================================
  Files         179      179            
  Lines       16293    16294     +1     
========================================
- Hits        15064    14438   -626     
- Misses       1229     1856   +627     

@rohitgr7 rohitgr7 merged commit 09cf167 into Lightning-AI:master Nov 11, 2021
awaelchli pushed a commit that referenced this pull request Nov 16, 2021
lexierule pushed a commit that referenced this pull request Nov 16, 2021
Raalsky pushed a commit to neptune-ai/pytorch-lightning that referenced this pull request Nov 23, 2021
@rohitgr7 rohitgr7 mentioned this pull request Feb 7, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better name for parameter in RichProgressBarTheme
5 participants