fix: friendly error messages for optional backend dependencies#343
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
113d79e to
d110ef2
Compare
|
I think this helps setup for a user -- does it make sense? Any thoughts on docling/vllm (leaving as draft until agreed) |
jakelorocco
left a comment
There was a problem hiding this comment.
I think putting this in start_session makes sense. If you are directly instantiating the backend, it's probably more clear what's going wrong.
- Wraps optional backend imports (hf, watsonx, litellm) in try/except blocks - Updates AGENTS.md with new coding standard - Adds reproduction script and PR description artifact
Signed-off-by: Nigel Jones <[email protected]>
12942fd to
e8b6b82
Compare
@jakelorocco Are you ok with where they are currently? they are in a method invoked only from start_session - it was the main init that it seemed would catch people out the most We can add more checks - even for any import, but I was also wary of adding too much clutter. What do you think about the vllm/docling aspect
(Rebased) |
|
I think the PR is good as is. We can make changes to the backends as well, but I think those errors are more intuitive since you would be directly importing the backend in those scenarios. I agree though; for the backends (vllm included), we should consider what this looks like at that level. And same for docling / other potential imports like that. |
|
Ok can go with this one, and I'll raise followups if I have a proposal on the others |
Misc PR
Type of PR
Description
Summary
Improves the user experience for optional backends (
hf,watsonx,litellm) by wrapping their imports intry/except ImportErrorblocks.Problem
Previously, if a user ran
start_session(backend="hf")without installingmellea[hf], they received a rawModuleNotFoundError: No module named 'outlines'. This was confusing for new users who didn't knowoutlinesis an internal dependency of the HuggingFace backend.Solution
This PR catches the
ImportErrorduring the backend resolution phase and raises a cleanerImportErrorthat explicitly tells the user which extra to install.Fixes
New Error Message:
Note on Other Dependencies
start_session(), so users cannot trigger this crash. To fix this, we would first need to add "vllm" support tobackend_name_to_class, and then wrap that new import in the sametry/exceptblock. That feels like a new feature so do it when adding?RichDocument, but requires refactoring top-level imports (lazy loading) to fix properly. Out of scope for this quick fix. Can add here for completeness but would mean a bigger change?pytest) will still raise standard errors.Testing
Verified in a clean virtual environment (Python 3.12) with no extra dependencies installed.
Test Script Output:
Click to see reproduction script (verify_fixes.py)