Skip to content

DOC Improve ProgressBar rich dependency note with link and install command#34189

Open
EdenRochmanSharabi wants to merge 2 commits into
scikit-learn:mainfrom
EdenRochmanSharabi:doc/progressbar-rich-link
Open

DOC Improve ProgressBar rich dependency note with link and install command#34189
EdenRochmanSharabi wants to merge 2 commits into
scikit-learn:mainfrom
EdenRochmanSharabi:doc/progressbar-rich-link

Conversation

@EdenRochmanSharabi
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Follow-up to #34168 (fixes #34166).

What does this implement/fix? Explain your changes.

Improves the ProgressBar docstring note added in #34168 by:

  • Adding a link to the rich documentation using standard RST syntax
  • Adding the pip install rich command so users know how to install it

Any other comments?

Documentation-only change. As suggested in #34184, contributing the improvement here instead.

Copy link
Copy Markdown
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Okay for the link, but we don't want to impose an install command. One can use conda for instance.

Comment thread sklearn/callback/_progressbar.py Outdated
Comment on lines +32 to +33
This callback requires `rich <https://rich.readthedocs.io>`_ to be installed.
It can be installed with ``pip install rich``.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
This callback requires `rich <https://rich.readthedocs.io>`_ to be installed.
It can be installed with ``pip install rich``.
This callback requires `rich <https://rich.readthedocs.io>`_ to be installed.

Copy link
Copy Markdown
Contributor

@AnneBeyer AnneBeyer left a comment

Choose a reason for hiding this comment

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

Hi @EdenRochmanSharabi, thank you, LGTM.

A side note (on what I was suggesting in the old PR, just in case you didn't know this):
When you go to the Files changed tab at the top of a PR, you can also add your suggestions as a comment to specific lines of code. So you could have added this as a suggestion to the existing PR instead of opening a follow-up one (at least as long as it wasn't already merged).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DOC Progressbar requires to install rich

3 participants