Skip to content

Commit 798ac76

Browse files
committed
fix: missing nice import errors and missing test scenarios
1 parent 4233559 commit 798ac76

8 files changed

Lines changed: 1116 additions & 1026 deletions

File tree

cli/alora/intrinsic_uploader.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@
1414

1515
try:
1616
import git
17-
from huggingface_hub import HfFolder, RepoUrl, create_repo, upload_file, upload_folder
17+
from huggingface_hub import (
18+
HfFolder,
19+
RepoUrl,
20+
create_repo,
21+
upload_file,
22+
upload_folder,
23+
)
1824
except ImportError as e:
1925
raise ImportError(
2026
"The 'm alora upload' command requires extra dependencies. "

cli/m.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,17 @@
88

99
try:
1010
import typer
11-
except ImportError:
12-
raise SystemExit(
11+
except ImportError as e:
12+
raise ImportError(
1313
"The 'm' CLI requires extra dependencies. "
1414
'Please install them with: pip install "mellea[cli]"'
15-
) from None
15+
) from e
1616

1717
from cli.alora.commands import alora_app
1818
from cli.decompose import app as decompose_app
1919
from cli.eval.commands import eval_app
2020
from cli.fix import fix_app
21-
from cli.serve.app import serve
21+
from cli.serve.commands import serve
2222

2323
cli = typer.Typer(name="m", no_args_is_help=True)
2424

@@ -36,10 +36,10 @@ def callback() -> None:
3636

3737
# Typer assumes that all commands are in the same file/module.
3838
# Use this workaround to separate out functionality. Can still be called
39-
# as if added with @cli.command() (ie `m serve` here).
39+
# as if added with @cli.command() (ie `m serve` here). If we don't use this
40+
# approach, we would have to use `m server <subcommand>` instead.
4041
cli.command(name="serve")(serve)
4142

42-
4343
# Add new subcommand groups by importing and adding with `cli.add_typer()`
4444
# as documented: https://typer.tiangolo.com/tutorial/subcommands/add-typer/#put-them-together.
4545
cli.add_typer(alora_app)

cli/serve/app.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,7 @@ async def endpoint(request: ChatCompletionRequest):
130130
return endpoint
131131

132132

133-
def serve(
134-
script_path: str = typer.Argument(
135-
default="docs/examples/m_serve/example.py",
136-
help="Path to the Python script to import and serve",
137-
),
138-
host: str = typer.Option("0.0.0.0", help="Host to bind to"),
139-
port: int = typer.Option(8080, help="Port to bind to"),
140-
):
133+
def run_server(script_path: str, host: str = "0.0.0.0", port: int = 8080):
141134
"""Serve a FastAPI endpoint for a given script."""
142135
module = load_module_from_path(script_path)
143136
route_path = "/v1/chat/completions"

cli/serve/commands.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
"""Typer command definition for ``m serve``.
2+
3+
Separates the CLI interface (typer annotations) from the server implementation
4+
(FastAPI, uvicorn) so that ``m --help`` works without the ``server`` extra installed.
5+
The heavy server dependencies are only imported when ``m serve`` is actually invoked.
6+
"""
7+
8+
import typer
9+
10+
11+
def serve(
12+
script_path: str = typer.Argument(
13+
default="docs/examples/m_serve/example.py",
14+
help="Path to the Python script to import and serve",
15+
),
16+
host: str = typer.Option("0.0.0.0", help="Host to bind to"),
17+
port: int = typer.Option(8080, help="Port to bind to"),
18+
):
19+
"""Serve a FastAPI endpoint for a given script."""
20+
from cli.serve.app import run_server
21+
22+
run_server(script_path=script_path, host=host, port=port)

mellea/backends/litellm.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,15 @@
77
from collections.abc import Callable, Coroutine, Sequence
88
from typing import Any, overload
99

10-
import litellm
11-
import litellm.litellm_core_utils
12-
import litellm.litellm_core_utils.get_supported_openai_params
10+
try:
11+
import litellm
12+
import litellm.litellm_core_utils
13+
import litellm.litellm_core_utils.get_supported_openai_params
14+
except ImportError as e:
15+
raise ImportError(
16+
"The LiteLLM backend requires extra dependencies. "
17+
'Please install them with: pip install "mellea[litellm]"'
18+
) from e
1319

1420
from ..backends import model_ids
1521
from ..core import (

mellea/backends/watsonx.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,15 @@
1010
from dataclasses import fields
1111
from typing import Any, overload
1212

13-
from ibm_watsonx_ai import APIClient, Credentials
14-
from ibm_watsonx_ai.foundation_models import ModelInference
15-
from ibm_watsonx_ai.foundation_models.schema import TextChatParameters
13+
try:
14+
from ibm_watsonx_ai import APIClient, Credentials
15+
from ibm_watsonx_ai.foundation_models import ModelInference
16+
from ibm_watsonx_ai.foundation_models.schema import TextChatParameters
17+
except ImportError as e:
18+
raise ImportError(
19+
"The Watsonx backend requires extra dependencies. "
20+
'Please install them with: pip install "mellea[watsonx]"'
21+
) from e
1622

1723
from ..backends import ModelIdentifier, model_ids
1824
from ..core import (

test/package/test_dependency_isolation.py

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@
6060
"docling": ["from mellea.stdlib.components.docs.richdocument import RichDocument"],
6161
"granite_retriever": [
6262
"import sentence_transformers",
63-
"import pyarrow",
63+
"from mellea.formatters.granite.retrievers.util import download_mtrag_corpus",
6464
"import elasticsearch",
6565
],
66-
"cli": ["import typer"],
67-
"server": ["import uvicorn", "import fastapi"],
66+
"cli": ["from cli.m import cli"],
67+
"server": ["from cli.serve.app import run_server"],
6868
"sandbox": ["import llm_sandbox"],
6969
"hooks": [
7070
"import cpex" # We directly import a non-Mellea class/package here since the hooks classes are defined regardless of if cpex is installed.
@@ -89,6 +89,11 @@
8989
# Meta-groups that compose other extras (no dedicated isolation test needed).
9090
META_GROUPS = {"backends", "all"}
9191

92+
# Extras whose IMPORTS entries all go through guarded mellea/cli modules
93+
# (i.e. ImportError will contain a mellea[<extra>] hint). Allows us to
94+
# check for a nice ImportError message.
95+
GUARDED_EXTRAS = {*BACKEND_EXTRAS, "server", "docling", "cli"}
96+
9297
# Extras that have a corresponding test_<name> function in this module.
9398
# Used to determine if we are missing tests for a given optional-dependency group.
9499
TESTED_EXTRAS = {
@@ -125,32 +130,38 @@ def _parse_optional_dependency_groups() -> set[str]:
125130
return set(data.get("project", {}).get("optional-dependencies", {}).keys())
126131

127132

128-
def _backend_fail_imports(exclude: set[str] | str = "") -> list[str]:
129-
"""Return backend import statements that should fail, excluding the given extras.
133+
def _normalize_exclude(exclude: set[str] | str = "") -> set[str]:
134+
"""Normalize an exclude argument to a set of extra names."""
135+
if isinstance(exclude, str):
136+
return {exclude} if exclude else set()
137+
return exclude
138+
139+
140+
def _backend_fail_imports(exclude: set[str] | str = "") -> list[tuple[str, str]]:
141+
"""Return ``(import_stmt, extra_name)`` pairs for backends expected to fail.
130142
131143
Collects import statements from all ``BACKEND_EXTRAS`` except those in
132144
``exclude``, and also excludes ``vllm`` on macOS since it is unsupported there.
145+
Each tuple pairs the import statement with the extra name so the checker
146+
script can verify the ``ImportError`` contains a ``mellea[<extra>]`` hint.
133147
134148
Args:
135149
exclude (set[str] | str): Backend extras to omit (their imports are
136150
expected to succeed). Pass a set of extra names, a single name, or
137151
an empty string to include all backends.
138152
139153
Returns:
140-
list[str]: Import statements that should raise ``ImportError`` in the
141-
isolated environment.
154+
list[tuple[str, str]]: Pairs of ``(import_statement, extra_name)``.
142155
"""
143-
if isinstance(exclude, str):
144-
exclude = {exclude} if exclude else set()
145-
extras = BACKEND_EXTRAS - exclude
156+
extras = BACKEND_EXTRAS - _normalize_exclude(exclude)
146157
if IS_MACOS:
147158
extras -= {"vllm"}
148-
return [stmt for name in sorted(extras) for stmt in IMPORTS[name]]
159+
return [(stmt, name) for name in sorted(extras) for stmt in IMPORTS[name]]
149160

150161

151162
def _build_check_script(
152163
should_succeed: list[str] = [],
153-
should_fail: list[str] = [],
164+
should_fail: list[tuple[str, str]] = [],
154165
flag_checks: list[tuple[str, str, bool]] = [],
155166
) -> str:
156167
"""Build a self-contained Python script that tests imports and prints failures.
@@ -161,7 +172,9 @@ def _build_check_script(
161172
162173
Args:
163174
should_succeed (list[str]): Import statements that must execute without error.
164-
should_fail (list[str]): Import statements that must raise ``ImportError``.
175+
should_fail (list[tuple[str, str]]): Tuples of
176+
``(import_statement, extra_name)`` — the import must raise
177+
``ImportError`` and the message must contain ``mellea[<extra_name>]``.
165178
flag_checks (list[tuple[str, str, bool]]): Tuples of
166179
``(module_path, attribute_name, expected_value)`` for boolean flag
167180
assertions (e.g. ``("mellea.telemetry.tracing", "_OTEL_AVAILABLE", True)``).
@@ -178,12 +191,16 @@ def _build_check_script(
178191
lines.append(f" failures.append(f'SHOULD SUCCEED: {stmt}: {{e}}')")
179192
lines.append("")
180193

181-
for stmt in should_fail:
194+
for stmt, extra in should_fail:
195+
hint = f"mellea[{extra}]"
182196
lines.append("try:")
183197
lines.append(f" {stmt}")
184198
lines.append(f" failures.append('SHOULD FAIL but succeeded: {stmt}')")
185-
lines.append("except ImportError:")
186-
lines.append(" pass")
199+
lines.append("except ImportError as e:")
200+
lines.append(f" if '{hint}' not in str(e):")
201+
lines.append(
202+
f" failures.append(f'MISSING HINT {hint} in: {stmt}: {{e}}')"
203+
)
187204
lines.append("except Exception:")
188205
lines.append(" pass # non-ImportError counts as unavailable")
189206
lines.append("")
@@ -209,7 +226,7 @@ def _build_check_script(
209226
def _run_check(
210227
extra: str | None = None,
211228
should_succeed: list[str] = [],
212-
should_fail: list[str] = [],
229+
should_fail: list[tuple[str, str]] = [],
213230
flag_checks: list[tuple[str, str, bool]] = [],
214231
) -> None:
215232
"""Build and run an import check script in an isolated uv environment.
@@ -223,7 +240,9 @@ def _run_check(
223240
extra (str | None): The optional-dependency extra to install
224241
(e.g. ``"hf"``). Pass ``None`` to install core mellea only.
225242
should_succeed (list[str]): Import statements that must execute without error.
226-
should_fail (list[str]): Import statements that must raise ``ImportError``.
243+
should_fail (list[tuple[str, str]]): Tuples of
244+
``(import_statement, extra_name)`` — must raise ``ImportError``
245+
with ``mellea[<extra_name>]`` in the message.
227246
flag_checks (list[tuple[str, str, bool]]): Tuples of
228247
``(module_path, attribute_name, expected_value)`` for boolean flag
229248
assertions.
@@ -232,7 +251,8 @@ def _run_check(
232251
ValueError: If any import statement appears in both ``should_succeed``
233252
and ``should_fail``.
234253
"""
235-
overlap = set(should_succeed) & set(should_fail)
254+
fail_stmts = {stmt for stmt, _ in should_fail}
255+
overlap = set(should_succeed) & fail_stmts
236256
if overlap:
237257
raise ValueError(
238258
f"Same imports in both should_succeed and should_fail: {overlap}"
@@ -286,10 +306,17 @@ def _inverted_flag_checks(extra: str) -> list[tuple[str, str, bool]]:
286306

287307

288308
def test_core_only() -> None:
289-
"""Core mellea with no extras: basic imports work, optional backends fail."""
309+
"""Core mellea with no extras: basic imports work, optional extras fail with hints."""
290310
_run_check(
291311
should_succeed=IMPORTS["core"],
292-
should_fail=_backend_fail_imports(exclude=""),
312+
should_fail=[
313+
*_backend_fail_imports(exclude=""),
314+
*[
315+
(stmt, name)
316+
for name in sorted(GUARDED_EXTRAS - BACKEND_EXTRAS)
317+
for stmt in IMPORTS[name]
318+
],
319+
],
293320
flag_checks=[
294321
*_inverted_flag_checks("telemetry"),
295322
*_inverted_flag_checks("hooks"),
@@ -298,7 +325,7 @@ def test_core_only() -> None:
298325

299326

300327
def test_hf() -> None:
301-
"""mellea[hf]: HuggingFace backend imports succeed, others fail."""
328+
"""mellea[hf]: HuggingFace backend imports succeed, others fail with hints."""
302329
_run_check(
303330
extra="hf",
304331
should_succeed=[*IMPORTS["core"], *IMPORTS["hf"]],
@@ -308,7 +335,7 @@ def test_hf() -> None:
308335

309336
@pytest.mark.skipif(IS_MACOS, reason="vllm is not supported on macOS")
310337
def test_vllm() -> None:
311-
"""mellea[vllm]: vLLM backend imports succeed, others fail."""
338+
"""mellea[vllm]: vLLM backend imports succeed, others fail with hints."""
312339
_run_check(
313340
extra="vllm",
314341
should_succeed=[*IMPORTS["core"], *IMPORTS["vllm"]],
@@ -317,7 +344,7 @@ def test_vllm() -> None:
317344

318345

319346
def test_litellm() -> None:
320-
"""mellea[litellm]: LiteLLM backend imports succeed, others fail."""
347+
"""mellea[litellm]: LiteLLM backend imports succeed, others fail with hints."""
321348
_run_check(
322349
extra="litellm",
323350
should_succeed=[*IMPORTS["core"], *IMPORTS["litellm"]],
@@ -326,7 +353,7 @@ def test_litellm() -> None:
326353

327354

328355
def test_watsonx() -> None:
329-
"""mellea[watsonx]: Watsonx backend imports succeed, others fail."""
356+
"""mellea[watsonx]: Watsonx backend imports succeed, others fail with hints."""
330357
_run_check(
331358
extra="watsonx",
332359
should_succeed=[*IMPORTS["core"], *IMPORTS["watsonx"]],
@@ -462,7 +489,7 @@ def test_checker_detects_should_succeed_failure() -> None:
462489

463490
def test_checker_detects_should_fail_that_succeeds() -> None:
464491
"""Checker script exits non-zero when a should_fail import actually works."""
465-
script = _build_check_script(should_fail=["import json"])
492+
script = _build_check_script(should_fail=[("import json", "fake")])
466493
result = _run_script_raw(script)
467494
assert result.returncode != 0
468495
assert "SHOULD FAIL but succeeded" in result.stderr
@@ -478,11 +505,21 @@ def test_checker_detects_wrong_flag_value() -> None:
478505
assert "FLAG sys.maxsize" in result.stderr
479506

480507

508+
def test_checker_detects_missing_hint() -> None:
509+
"""Checker script exits non-zero when ImportError lacks the expected install hint."""
510+
# Raise an ImportError without the expected hint substring
511+
script = _build_check_script(
512+
should_fail=[("import no_such_module_xyz", "fake_extra")]
513+
)
514+
result = _run_script_raw(script)
515+
assert result.returncode != 0
516+
assert "MISSING HINT mellea[fake_extra]" in result.stderr
517+
518+
481519
def test_checker_passes_when_all_correct() -> None:
482520
"""Checker script exits zero when all checks pass."""
483521
script = _build_check_script(
484522
should_succeed=["import json", "from os.path import join"],
485-
should_fail=["import no_such_module_xyz"],
486523
flag_checks=[("sys", "maxsize", sys.maxsize)], # type: ignore[list-item]
487524
)
488525
result = _run_script_raw(script)

0 commit comments

Comments
 (0)