rich
rich copied to clipboard
Introduce the uninstall method
Type of changes
- [ ] Bug fix
- [x] New feature
- [ ] Documentation / docstrings
- [ ] Tests
- [ ] Other
Checklist
- [ ] I've run the latest black with default args on new code.
- [ ] I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
- [ ] I've added tests for new code.
- [ ] I accept that @willmcgugan may be pedantic in the code review.
Description
Related #2461, #1947
This is a very rough draft but it's a working solution. List of questions that I have
- Is manual testing enough? I am not sure how can I write an automated test for this and need some help
- Previously when running
installfor IPython, it returns asys.excepthookwhich isn't useful, because it's just a pointer to the object and the old hooks that we want to preserve are gone. - It's also not very consistent now with this change as for Python path, it returns a hook but for IPython path it returns a list of method which need to be recover when
uninstall - Should a global variable or a stateful class be used? This will make it easier to use
rich.traceback.uninstall(), but it will also be a breaking change so I haven't implement in this way yet, would love to get some feedback.
Manual Test
Python
IPython
Please describe your changes here. If this fixes a bug, please link to the issue, if possible. I have tested this on Github workspace, you can use this script to verify the behavior.
python
import rich.traceback
hooks = rich.traceback.install()
rich.traceback.uninstall(hooks)
error # trigger error
ipython
import rich.traceback
hooks = rich.traceback.install()
rich.traceback.uninstall(hooks)
error # trigger error
I'd accept a PR for an
uninstallfunction. Originally posted by @willmcgugan in https://github.com/Textualize/rich/issues/2461#issuecomment-1211740277
Pinging for review/comments, do you think this is still valuable?
for the sanity of everyone everywhere please merge this
@qrdlgit Could I ask you to read over the code of conduct and reflect on your approach and language: https://github.com/Textualize/rich/blob/master/CODE_OF_CONDUCT.md
I'm sure whatever issues you've run into, due to the decisions made by the authors of the libraries you've chosen to install, are frustrating; but using Rich issues to vent at the authors and maintainers of Rich isn't going to solve anything.
Related https://github.com/Textualize/rich/issues/2461
Hey @davep , any chance that I can get some feedback on the PR?
@noklam Not something I'm in a position to help with I'm afraid.
You are hooking into the users environment and changing it so that you live beyond the lifetime of the installed packages / processes the user wants to use.
This is nonsense.
Your belligerent tone makes me disinclined to unburden you of your ignorance. If you want help, post a new issue (do not reply to this issue). But bear in mind you are talking to real people who have worked very hard to bring you some amazing software for free.
Run source $VENV black --check . would reformat rich/tree.py
Oh no! 💥 💔 💥 1 file would be reformatted, 189 files would be left unchanged. make: *** [Makefile:6: format-check] Error 1 Error: Process completed with exit code 2.
I see a linting error but doesn't seem to be related to my change.