-
Notifications
You must be signed in to change notification settings - Fork 3
Slim core dependencies; promote openai to base install #208
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,11 @@ | ||
| cachetools | ||
| IPython | ||
| loguru | ||
| opencv-python>=4.8.0 | ||
| pandas | ||
| openai>=1.0.0 | ||
| Pillow>=10.2.0 | ||
| pydantic>=2.5,<3 | ||
| pydantic_core>=2.23.4 | ||
| requests | ||
| rich | ||
| tabulate | ||
| tenacity | ||
| tqdm | ||
| typer>=0.9.0 | ||
| vlmrun-hub>=0.1.28 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,46 +1,77 @@ | ||
| """Tests for verifying correct installation of optional dependencies.""" | ||
| """Tests for verifying optional dependency handling.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import builtins | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| from vlmrun.client.exceptions import DependencyError | ||
| from vlmrun.common import dependencies | ||
|
|
||
|
|
||
| def _block_import(monkeypatch, module_name: str) -> None: | ||
| for key in list(sys.modules): | ||
| if key == module_name or key.startswith(f"{module_name}."): | ||
| monkeypatch.delitem(sys.modules, key, raising=False) | ||
|
|
||
| @pytest.mark.skip(reason="Temporarily skipped as requested") | ||
| def test_base_dependencies(): | ||
| """Verify base installation has no optional dependencies.""" | ||
| with pytest.raises(ImportError): | ||
| import cv2 # noqa: F401 | ||
| real_import = builtins.__import__ | ||
|
|
||
| with pytest.raises(ImportError): | ||
| import pypdfium2 # noqa: F401 | ||
| def mock_import(name, globals=None, locals=None, fromlist=(), level=0): | ||
| blocked = ( | ||
| name == module_name | ||
| or name.startswith(f"{module_name}.") | ||
| or (fromlist and module_name in fromlist) | ||
| ) | ||
| if blocked: | ||
| raise ImportError(f"No module named '{module_name}'") | ||
| return real_import(name, globals, locals, fromlist, level) | ||
|
|
||
| monkeypatch.setattr(builtins, "__import__", mock_import) | ||
|
|
||
| @pytest.mark.skip(reason="Temporarily skipped as requested") | ||
| def test_video_dependencies(): | ||
| """Verify video dependencies are available.""" | ||
| import cv2 # noqa: F401 | ||
| import numpy as np # noqa: F401 | ||
|
|
||
| # Verify we can import and get versions | ||
| assert cv2.__version__, "cv2 version should be available" | ||
| assert np.__version__, "numpy version should be available" | ||
| def test_require_openai_suggestion(monkeypatch): | ||
| """OpenAI is a core dependency; errors should point at base install.""" | ||
| _block_import(monkeypatch, "openai") | ||
| with pytest.raises(DependencyError) as exc_info: | ||
| dependencies.require_openai() | ||
| assert "pip install vlmrun" in exc_info.value.suggestion | ||
| assert "[openai]" not in exc_info.value.suggestion | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="Temporarily skipped as requested") | ||
| def test_doc_dependencies(): | ||
| """Verify doc dependencies are available.""" | ||
| import pypdfium2 # noqa: F401 | ||
| @pytest.mark.parametrize( | ||
| ("require_fn", "module_name", "extra"), | ||
| [ | ||
| (dependencies.require_pandas, "pandas", "all"), | ||
| (dependencies.require_numpy, "numpy", "video"), | ||
| (dependencies.require_cv2, "cv2", "video"), | ||
| (dependencies.require_ipython_html, "IPython", "all"), | ||
| (dependencies.require_pypdfium2, "pypdfium2", "doc"), | ||
| ], | ||
| ) | ||
| def test_optional_dependency_errors(require_fn, module_name, extra, monkeypatch): | ||
| """Missing optional deps should raise DependencyError with install hints.""" | ||
| _block_import(monkeypatch, module_name) | ||
| with pytest.raises(DependencyError) as exc_info: | ||
| require_fn() | ||
| assert f"vlmrun[{extra}]" in exc_info.value.suggestion | ||
|
|
||
| # Verify we can import and get version | ||
| assert pypdfium2.__version__, "pypdfium2 version should be available" | ||
|
|
||
| def test_markdown_table_to_dataframe_requires_pandas(monkeypatch): | ||
| """MarkdownTable.to_dataframe should lazy-load pandas.""" | ||
| def _raise_pandas(): | ||
| raise DependencyError( | ||
| message="pandas is not installed", | ||
| suggestion="Install it with `pip install vlmrun[all]`", | ||
| ) | ||
|
|
||
| @pytest.mark.skip(reason="Temporarily skipped as requested") | ||
| def test_all_dependencies(): | ||
| """Verify all dependencies are available.""" | ||
| import cv2 # noqa: F401 | ||
| import numpy as np # noqa: F401 | ||
| import pypdfium2 # noqa: F401 | ||
| monkeypatch.setattr("vlmrun.client.types.require_pandas", _raise_pandas) | ||
| from vlmrun.client.types import MarkdownTable, TableHeader | ||
|
|
||
| # Verify we can import and get versions | ||
| assert cv2.__version__, "cv2 version should be available" | ||
| assert np.__version__, "numpy version should be available" | ||
| assert pypdfium2.__version__, "pypdfium2 version should be available" | ||
| table = MarkdownTable( | ||
| headers=[TableHeader(id="col1", column=0, name="Column 1")], | ||
| data=[{"col1": "value"}], | ||
| ) | ||
| with pytest.raises(DependencyError): | ||
| table.to_dataframe() |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Gateway availability check crashes instead of reporting status The gateway liveness check reads a success flag that does not exist on the new HTTP library's response object ( httpx→requests migration missed the response API difference
(Refers to line 231) Was this helpful? React with 👍 or 👎 to provide feedback. |
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.
🟡 Documentation still claims chat completions raise a missing-dependency error
The completions helpers still document a missing-dependency error (
Raises: DependencyErroratvlmrun/client/agent.py:305andvlmrun/client/agent.py:350) even though the OpenAI SDK is now imported unconditionally at module level, so the docs no longer match behavior.Impact: Readers are told about an error condition that can never occur, which is misleading.
Docstrings not updated with the import change
agent.pynow doesfrom openai import AsyncOpenAI, OpenAIat the top (vlmrun/client/agent.py:22) and the try/except that raisedDependencyErrorwas removed from bothcompletionsandasync_completions. AGENTS.md requires docstrings/docs be updated when the implementation deviates; theRaises:sections should be dropped (or replaced with anImportErrornote).(Refers to lines 304-306)
Was this helpful? React with 👍 or 👎 to provide feedback.