Skip to content

[SC-19101] Fix list_tests crash on installs without optional extras - #573

Merged
cachafla merged 3 commits into
mainfrom
cachafla/list-tests-optional-imports
Oct 22, 2026
Merged

cachafla merged 3 commits into
mainfrom
cachafla/list-tests-optional-imports

Conversation

@cachafla

@cachafla cachafla commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request Description

What and why?

vm.tests.list_tests(filter=..., pretty=True) crashed on any install without the nlp/llm extras (e.g. pip install "validmind[xgboost]", which the quickstart notebooks use):

LoadTestError: Unable to load test 'validmind.model_validation.embeddings.StabilityAnalysisTranslation' from validmind test provider
ModuleNotFoundError: No module named 'transformers'

list_tests loads every test before filtering, and _load_tests only skips tests that raise MissingDependencyError. Three spots imported optional packages outside a guard, so they raised a plain ModuleNotFoundError and aborted the whole listing, whatever the filter:

  • embeddings/StabilityAnalysisTranslation.py: transformers, with no guard
  • all 10 ragas/* tests: datasets was imported above the existing ragas guard
  • ragas/utils.py: langchain_core. This was hidden behind the datasets failure and surfaced once that one was fixed

Each import now sits in the usual try/except ImportError block and raises MissingDependencyError with the right extra, so these tests are skipped the same way the other optional-dependency tests are.

Per review, there's also a fallback for the next unguarded import: _load_tests now skips any test whose import raises ModuleNotFoundError and logs one warning naming the missing packages (e.g. Skipping tests that import packages that aren't installed: `nltk` (2 tests); `statsmodels` (16 tests)). On a fresh [xgboost] install that covers 16 statsmodels tests and 2 nltk tests that have no guard. The core CI leg now runs a list_tests smoke check, and a unit test covers the fallback.

Also from review: jinja2 is now a declared core dependency. import validmind needs it (vm_models/result/utils.py imports it directly, and pandas' Styler used by format_dataframe requires it), but it was only installed transitively through extras, so a plain pip install validmind couldn't be imported outside Jupyter. The core CI leg no longer installs jinja2 by hand, so it exercises the bare wheel.

How to test

In an env with only the base package (+ xgboost):

import validmind as vm
vm.tests.list_tests(filter="classification", pretty=True)  # previously crashed

Ran locally:

  • Fresh venv with validmind[xgboost] + jinja2. Before the fallback: 165 load, 47 skipped, 18 crash (all statsmodels/nltk). After: list_tests(filter="classification") returns 98 tests with one warning, and pretty=True works.
  • Bare wheel (no extras, no manual jinja2) in a fresh venv: jinja2 comes in as a dependency, import validmind works, list_tests(filter="classification") returns 98 tests, and pretty=True works.
  • Full-extras env: all 11 touched tests still load.
  • tests.test_validmind_tests_module: 13 OK, including the new fallback test, which fails without the load.py change. tests/unit_tests/model_validation/ragas/test_utils.py: 2 passed. black/isort/flake8 clean.

CI smoke check: the core leg uses list_tests(filter="classification", pretty=False). It needs a filter: with no filter/task/tags and pretty=False, list_tests returns the IDs without importing any test, so it would never catch this.

What needs special review?

The ragas guard now matches "ragas" or "datasets" in the error message and lists both in required_dependencies.

The ModuleNotFoundError fallback logs one aggregated warning per list_tests call rather than one per test, to keep notebooks on minimal installs readable. Per-test details are logged at debug.

Dependencies, breaking changes, and deployment notes

Adds jinja2 (>=3.1.2) to core dependencies (pandas 2.x's minimum for Styler). uv.lock only gains the jinja2 edges.

Release notes

Fixed vm.tests.list_tests() failing with a LoadTestError when optional dependencies such as transformers or datasets weren't installed. Tests that need those extras are now skipped from the listing instead. jinja2 is now installed with the core package, so import validmind works after a plain pip install validmind.

Checklist

  • What and why
  • Screenshots or videos (Frontend)
  • How to test
  • What needs special review
  • Dependencies, breaking changes, and deployment notes
  • Labels applied
  • PR linked to Shortcut
  • Unit tests added (Backend)
  • Tested locally
  • Documentation updated (if required)
  • Environment variable additions/changes documented (if required)

🤖 Generated with Claude Code

list_tests() loads every test before filtering and only skips tests that
raise MissingDependencyError. StabilityAnalysisTranslation (transformers),
the ragas tests (datasets) and ragas/utils.py (langchain_core) imported
optional packages outside a guard, so a plain ModuleNotFoundError aborted
the whole listing on installs without the nlp/llm extras.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cachafla cachafla added the bug Something isn't working label Oct 22, 2026
@cachafla
cachafla requested a review from juanmleng October 22, 2026 18:20

@juanmleng juanmleng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: #573 at 36f5d6d (merge-base e4b744b)

Verdict: Merge with recommendations

Computed by the calibrated-review skill, run by @juanmleng.

Summary

  • What changed: Moves the optional transformers, datasets and langchain_core imports in 12 test modules inside the usual MissingDependencyError guard, so list_tests() skips those tests instead of crashing.
  • Result: The new guards are correct, but on a clean validmind[xgboost] install list_tests() still crashes, on 18 other tests.
  • Main thing to know: The PR's "none crash" result depends on statsmodels and nltk already being installed. Colab preinstalls them, but a fresh local venv does not have them.
  • Next step: Add one fallback in _load_tests (FN1) and a list_tests() smoke check to the existing core CI leg (FN2).

Fix now

FN1

  • What: On a fresh local venv with pip install "validmind[xgboost]", vm.tests.list_tests(filter="classification") still raises LoadTestError, the symptom this PR sets out to fix. 16 tests import statsmodels (in the stats extra) and 2 import nltk (in the nlp extra) at module top level with no guard.
  • Why: It is the PR's stated bug, so it is worth closing here. It is not blocking because it fails loudly and no worse than on main.
  • Where: validmind/tests/load.py:255-259 (_load_tests re-raises anything that is not MissingDependencyError). Examples of the unguarded imports:
    • validmind/tests/data_validation/ADF.py:8
    • validmind/tests/model_validation/embeddings/StabilityAnalysisSynonyms.py:8
  • Suggested fix: In _load_tests, also skip when e.original_error is a ModuleNotFoundError, and log it at warning rather than debug, so a genuinely broken import in a core test is still visible. That one guard covers all 18 tests and the next unguarded import someone adds. Keep the per-module guards this PR adds, because they give the better install message.

FN2

  • What: No CI job calls list_tests() without extras installed, so nothing caught this bug and nothing will catch the next one.
  • Why: The core leg of dependency-testing.yaml already installs the bare wheel (plus jinja2), so this is a one-line addition.
  • Where: .github/workflows/dependency-testing.yaml, core branch of the "Install and test via pip artifacts" step.
  • Suggested fix: python -c 'import validmind as vm; vm.tests.list_tests(pretty=False)'. It fails on this PR's head today and passes once FN1 lands.

Fix later

FL1. A plain pip install validmind cannot be imported outside Jupyter. validmind/tests/load.py:24 imports pandas' Styler, which requires jinja2, and jinja2 is not a declared dependency. The CI core leg already works around this with a comment. Anyone using the library from a script or a fresh venv hits ImportError: Missing optional dependency 'Jinja2' on import validmind. It is not blocking because it predates this PR. Fix: declare jinja2 in core dependencies, or import Styler lazily. Suggested owner: library maintainers.

Accepted risks

  • What I checked: whether the "datasets" in str(e) match is too loose or too narrow.

  • Why it is acceptable: It matches No module named 'datasets'. Any other ImportError is re-raised, the same as the existing ragas branch.

  • What I checked: the ragas tests import .utils above their own try, so the langchain_core guard fires before the ragas/datasets one.

  • Why it is acceptable: It still raises MissingDependencyError with extra="llm". Only the package named in the message changes.

Verification performed

  • Clean venv from the PR head, base package + jinja2, loading every ID from _list_test_ids() → total 230 ok 165 skip 47 crash 18
    • all 18 crashes are No module named 'statsmodels' or 'nltk'
  • Clean venv from the PR head with the [xgboost] extra + jinja2, same loop → crash 18, the same 18 tests
  • Same [xgboost] venv after pip install statsmodels nltk → ok 187 skip 43 crash 0, and list_tests(filter="classification") returns 106 tests
  • Clean venv without jinja2 → import validmind fails (FL1)
  • PR body claims:
    • "37 skipped, none crash, classification returns 109" → reproduced only with statsmodels and nltk present; my counts differ (43 / 106), which is consistent with a different environment
    • "Full-extras env: all 11 touched tests still load" → NOT CHECKED
    • "tests.test_validmind_tests_module 12 OK, ragas test_utils.py 2 passed, black/isort/flake8 clean" → NOT CHECKED

cachafla and others added 2 commits October 22, 2026 13:17
A fresh `validmind[xgboost]` install still crashed in list_tests() on 18
tests that import statsmodels or nltk without a guard. _load_tests now
skips any test whose import raises ModuleNotFoundError and logs one
warning naming the missing packages, so a broken import stays visible.

Add a list_tests smoke check to the core CI leg, using a filter so every
test is actually imported, plus a unit test for the fallback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`import validmind` needs jinja2: vm_models/result/utils.py imports it
directly and pandas' Styler (format_dataframe, Styler test outputs)
requires it. It was only installed transitively through optional
extras, so a plain `pip install validmind` failed to import outside
Jupyter. Drop the core CI leg's manual jinja2 install so that leg
exercises the bare wheel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cachafla

Copy link
Copy Markdown
Contributor Author

@juanmleng thanks for the review, all of it is addressed:

  • FN1 (7028bf8): _load_tests now skips tests whose import raises ModuleNotFoundError, and keeps the per-module guards. One difference from your suggestion: instead of a warning per test, it logs a single warning per call that names the missing packages, e.g. Skipping tests that import packages that aren't installed: `nltk` (2 tests); `statsmodels` (16 tests). Otherwise a fresh [xgboost] install would print 18 warning lines every time a notebook calls list_tests. Per-test details go to debug. A new unit test covers the fallback, and it fails without the change.
  • FN2 (7028bf8): the core leg now runs a list_tests smoke check. I used list_tests(filter="classification", pretty=False) rather than list_tests(pretty=False): with no filter, task or tags and pretty=False, list_tests returns the IDs early without importing any test, so it wouldn't catch this.
  • FL1 (9958621): jinja2 (>=3.1.2) is now a core dependency. It's imported directly in vm_models/result/utils.py and needed by Styler. The core leg no longer installs it by hand, so it tests the bare wheel.

Verified in fresh venvs:

  • [xgboost]: before, 18 crashes. After, list_tests(filter="classification") returns 98 tests and pretty=True works.
  • Bare wheel with no extras: jinja2 is installed, and import validmind and list_tests both work.

CI is running on the new commits.

@juanmleng juanmleng changed the title Fix list_tests crash on installs without optional extras [SC-19101] Fix list_tests crash on installs without optional extras Oct 22, 2026
@juanmleng

Copy link
Copy Markdown
Contributor

Thanks @cachafla, checked all three in fresh venvs: bare and [xgboost] both list 98 classification tests now, pretty=True works, and jinja2 comes in with the bare wheel. Good catch on the list_tests early return; my suggested smoke check would have tested nothing. New unit test fails without the load.py change. LGTM once CI finishes.

@cachafla
cachafla merged commit 28233e9 into main Oct 22, 2026
28 checks passed
@cachafla
cachafla deleted the cachafla/list-tests-optional-imports branch October 22, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants