-
-
Notifications
You must be signed in to change notification settings - Fork 624
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 typer.set_rich_*()
to allow Rich formatting to be disabled
#647
base: master
Are you sure you want to change the base?
Conversation
Implements a feature suggested by Anthony Rafidison (anthonyraf) in issue tiangolo#622 to optionally disable Rich output. Fix tiangolo#578, fix tiangolo#622
Since the internal state of `_is_rich_enabled` in `typer/core.py` exhibits global behavior with respect to all `Typer` instances, it is more consistent to refactor `typer.Typer(rich_enable=<bool>)` to `typer.enable_rich(<bool>)`.
Also, add a paragraph to the documentation.
📝 Docs preview for commit 67fad4b at: https://9fddef8a.typertiangolo.pages.dev |
📝 Docs preview for commit b0abe95 at: https://2e18d008.typertiangolo.pages.dev |
Feature/enable rich
📝 Docs preview for commit f265dc2 at: https://28e342bf.typertiangolo.pages.dev |
Remove obsolete function
📝 Docs preview for commit 87a9ce7 at: https://343d25f2.typertiangolo.pages.dev |
Update docs to reflect latest Rich output changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typer.enable_rich(False)
I guess you mean typer.enable_rich_help(False)
since that's what is in the code.
I ran the tests locally, and checked against my own example script. Everything works as advertised. The implementation is also straightforward.
That said, I think the functions should be called disable_rich_*
since enable is already the default behaviour, so in the enable_rich_*
form it will always be a double negative. Which is a bit weird.
Thanks for the review. I'll rename it as you suggested. I was thinking the same way about my previous naming choice anyway shortly after I published the PR but wanted to wait for a response before touching the code again. I don't know why I even named it that way to begin with. ;-) |
Is there any plan on having this merged and released? It's a really valuable contribution as not scenarios where rich and typer are installed mean users want to use rich with typer. |
📝 Docs preview for commit 42cf404 at: https://f8030773.typertiangolo.pages.dev |
I renamed the two functions and the thereby affected variables to be more semantically precise and more specific to their purpose. For the exposed functions I settled on the form: |
* Rename symbols to be more specific * Alias function with its namespace of origin * Auto format after rename/refactor * Rename symbols for better semantic precision * Update docs to new nomenclature used in PR tiangolo#647 --------- Co-authored-by: Jacob Kochems <462684-Kochems@users.noreply.gitlab.com>
📝 Docs preview for commit 2ade3dc at: https://18993260.typertiangolo.pages.dev |
Hello everyone, |
Since there has been no commits since July 30 I guess this PR has not been noticed by the maintainers yet.
@coreegor No, I can not. Since I'm not a maintainer I don't have the rights to do so. |
@JacobKochems So you can't set @tiangolo as the assignee ? |
No, I can't set anyone as assignee. Not myself and certainly not tiangolo. Nor would I want to. We are all guests here and it isn't my place to assign any task to anybody in this repository. |
@JacobKochems Ok, I agree. But is there any correct way to attract the attention of the maintainer to this request? |
@coreegor Since you already `@ed´ him I would assume that he already got an email notification but either didn't have time to respond yet or he time boxes his work on a given project at certain intervals. If you need this feature rather sooner than later, I see two options:
I'd prefer option 1. We all have to navigate a world where seemingly each and everything is competing for our attention. There is probably a reason why the last commit is from July 30. |
📝 Docs preview for commit 6a7735e at: https://576f736d.typertiangolo.pages.dev |
To all following this PR: The original motivation for this PR is the issue #646 for which this is a possible workaround. However, I'd rather like to see that original issue addressed as well. If some of you could review/replicate it, it would be much appreciated. |
typer.set_rich_*()
to allow Rich formatting to be disabledtyper.set_rich_*()
to allow Rich formatting to be disabled
📝 Docs preview for commit c408a39 at: https://ca1e8013.typertiangolo.pages.dev |
@JacobKochems I can replicate #646 with the latest Rich (13.7.0). To disable Rich though, I am doing the following: import typer.core
typer.core.rich = None
... |
This is working well, thanks |
Thank you for this very simple workaround. Recently, yet another effort related to issues #578 and #622 has been made: #726. This in combination with the kwarg pretty_exceptions_enable=False of |
I'd also like to add a vote for and option that allows globally disabling rich. Just because rich is installed and available doesn't always mean it's appropriate to render rich output from the cli. I'd like to not have to revert to disabling features piecemeal e.g. |
📝 Docs preview for commit fc18ef2 at: https://01e2c600.typertiangolo.pages.dev |
📝 Docs preview for commit 65ddbf0 at: https://667e652b.typertiangolo.pages.dev |
📝 Docs preview for commit 3f9bc07 at: https://15654bb2.typertiangolo.pages.dev |
After some updates from tiangolo's master branch I needed to resolve a merge conflict, which worked out. However, suddenly some checks are failing. I tried to rerun the test suite locally and ran into several issues: Commands and scripts are missing:
The test script provided by tiangolo does not work anymore:
Maybe pytest was updated but not the script? |
Implements a feature similar to the one suggested by @anthonyraf in issue #622 to optionally disable Rich output.
Two simple test cases and a section of documentation has also been added.
It affects all instances of
Typer
globally.The code from the test cases illustrates how to use it:
Resolves #578, resolves #622
Bonus: It is also a workaround for #646.