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

[INTEGRATION] JupyterLab integration due to upstream dependency constraint on Rich #3407

Closed
RobFirth opened this issue Jul 14, 2023 · 2 comments · Fixed by #3409
Closed

[INTEGRATION] JupyterLab integration due to upstream dependency constraint on Rich #3407

RobFirth opened this issue Jul 14, 2023 · 2 comments · Fixed by #3409
Assignees

Comments

@RobFirth
Copy link

Describe the bug
Tracebacks in jupyter notebooks in project virtual environment with argilla are broken as a result of upstream bug in old version of rich. Version constraint on rich = "<=13.0.1", this behaviour was fixed in rich==v13.3.1.

import argilla as rg

def f(x):
    return x/0

f(42)

returns:

╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /tmp/ipykernel_1885/2875885747.py:1 in <module>                                                  │
│                                                                                                  │
│ [Errno 2] No such file or directory: '/tmp/ipykernel_1885/2875885747.py'                         │
│                                                                                                  │
│ /tmp/ipykernel_1885/3374322498.py:2 in f                                                         │
│                                                                                                  │
│ [Errno 2] No such file or directory: '/tmp/ipykernel_1885/3374322498.py'                         │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
ZeroDivisionError: division by zero

Expected behaviour
the traceback gets displayed correctly.

Environment:

  • Argilla Version [e.g. 1.12.1]: 1.12.1
  • rich: 13.0.1

Additional context

This bug is described in the following issues:

This doesn't affect my Argilla workflow other than making it a total pain to debug issues in notebooks! Hopefully a version bump isn't too onerous - it would be a massive QOL improvement.

suggested fix
Version bump rich e.g. rich<=13.3.1

@tomaarsen tomaarsen self-assigned this Jul 14, 2023
@tomaarsen
Copy link
Contributor

Hello!

When we integrated rich in #2350, I specifically set rich = "<=13.0.1" because rich==13.1.0 was failing for me with this error. At that time, 13.1.0 was the most recent version.

Now I've installed the most recent version (13.4.2), and the aforementioned issue does not occur anymore. Because of this, I think we can lift the "<=13.0.1" restriction without causing crashes for users.

I'll make a PR for this.

  • Tom Aarsen

@RobFirth
Copy link
Author

Hello!

When we integrated rich in #2350, I specifically set rich = "<=13.0.1" because rich==13.1.0 was failing for me with this error. At that time, 13.1.0 was the most recent version.

Now I've installed the most recent version (13.4.2), and the aforementioned issue does not occur anymore. Because of this, I think we can lift the "<=13.0.1" restriction without causing crashes for users.

I'll make a PR for this.

  • Tom Aarsen

Thanks for this @tomaarsen - I was hoping that this was the reason for pinning the version and it wouldn't impact on anything elsewhere. Cheers for the speed 🚀

tomaarsen added a commit that referenced this issue Jul 14, 2023
Closes #3407

Hello!

# Description

List version restriction of `rich`. This restriction of `<= 13.0.1` was
introduced in #2350 due to a
[bug](Textualize/rich#2800 (comment))
in the version that was most recent back then: version 13.1.0.
However, the issue has been resolved as of the current most recent
version: 13.4.2.
Additionally, #3407 suggests allowing at least up to 13.3.1.

I think the best solution is just to let go of the version restriction
and let pip/conda/poetry install the most recent version.

**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [x] Bug fix

**How Has This Been Tested**

I installed the most recent `rich` and experimented a bit with some
scripts by making them crash etc.

**Checklist**

- [ ] I added relevant documentation
- [ ] follows the style guidelines of this project
- [x] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

I'm of the opinion that this is not worthy of a changelog entry.

---

- Tom Aarsen
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 a pull request may close this issue.

2 participants