Skip to content

FIX objective: guard heavy imports so benchopt can read metadata#50

Closed
felixdivo wants to merge 1 commit into
benchopt:mainfrom
felixdivo:fix/objective-import-guard
Closed

FIX objective: guard heavy imports so benchopt can read metadata#50
felixdivo wants to merge 1 commit into
benchopt:mainfrom
felixdivo:fix/objective-import-guard

Conversation

@felixdivo

Copy link
Copy Markdown
Contributor

benchopt_dev CI fails with Patterns ['simulated'] did not match any dataset.

Cause: objective.py imports benchmark_utils.metrics, which imports sklearn at
module level. benchopt reads the objective's metadata in an env without the
objective's requirements installed; on that failed import it returns a
FailedImport stub (no test_config, test_dataset_name=None), which now
resolves to the non-existent simulated dataset.

The simulated fallback was introduced in benchopt/benchopt#951 (default
test_dataset_name 'simulated'None plus a ['simulated'] fallback),
on top of the test_config resolution added in benchopt/benchopt#942.

Fix: wrap the metrics import in safe_import_context so the module imports for
metadata without sklearn. The metric names are only used in method bodies.

🤖 Generated with Claude Code

@felixdivo felixdivo requested a review from tomMoral June 13, 2026 22:38
@felixdivo felixdivo self-assigned this Jun 13, 2026
@felixdivo felixdivo added the bug Something isn't working label Jun 13, 2026
@felixdivo felixdivo marked this pull request as draft June 13, 2026 22:49
…lvable

`benchopt test` (benchopt_dev CI) failed with
`Patterns ['simulated'] did not match any dataset`. Two issues, both from
benchopt importing the benchmark in environments without the objective's
requirements installed:

1. Dataset resolution under skip_import. When benchopt builds the test env it
   loads the objective under `skip_import_ctx` — it never imports the module,
   it statically AST-parses class attributes, and only extracts attributes that
   exist on `BaseObjective`. `test_config` is not a base attribute, so our
   `test_config['dataset']['name']` was invisible and resolution fell back to
   the non-existent `simulated` dataset. Fix: also set `test_dataset_name`
   (a base attribute, so it IS extracted statically) to a real dataset.

2. sklearn import at module load. `benchmark_utils.metrics` imported sklearn at
   module top level, so importing objective.py (and the AD datasets, which pull
   `AD_METRICS` for its keys) failed when sklearn was absent. Fix: import
   sklearn lazily inside the metric functions; the registries and metric names
   are now available with only numpy installed. The functions still run with
   sklearn present (it is an objective requirement, installed for real runs).

This supersedes the earlier `safe_import_context` attempt, which made benchopt
flag the whole objective as a failed import under `skip_import_ctx`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@felixdivo felixdivo force-pushed the fix/objective-import-guard branch from 1df5f08 to 8ebb5d4 Compare June 14, 2026 18:30
@felixdivo

Copy link
Copy Markdown
Contributor Author

This should not be needed once benchopt/benchopt#958 is merged upstream.

@felixdivo felixdivo closed this Jun 15, 2026
@tomMoral

Copy link
Copy Markdown
Member

Indeed! sorry for the mess, I was making a lot of change to make this more robust, I thought I had a test but it was not testing what I thought... ^^

Thanks for the ping, this helped identifying what was missing. :)
I will try to move on agent's skills soon + review doc.

@felixdivo felixdivo deleted the fix/objective-import-guard branch June 15, 2026 09:27
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