drevalpy v2 - #460
Draft
nictru wants to merge 436 commits into
Draft
Conversation
The normalizer stopped taking recipe strings when expansion moved to the recipe module, but its fallback error still offered one as a valid form.
A single-drug predictor had to declare routing_drug_featurizer='identity', and registration rejected any other value. The class variable was therefore a knob wired to a constant: three further places hardcoded 'identity' anyway. The scope now implies the routing featurizer, so the declaration, its two subclass copies and the registration check are gone, and one named constant serves the block lookups, the pairing error and the model_id omission. What was a lookup returning a name is now the question it always asked, needs_identity_ drug_routing, keeping the carve-out for predictors that consume no drug features.
A predictor could declare requires_drug_featurizer=False to be composed without one, but nothing exercised the exemption: both baselines that claimed it ship with drug_featurizer set anyway, and the runtime branch that skipped materializing drug features was unreachable, since every predictor reaching it required them. Feature-based predictors now always name both featurizers, and the ones that read only the cell-line side simply ignore the other. Feature-free predictors still forbid both, which is what the single-drug routing check now consults in place of the deleted flag.
The name only decorated a debug log and two error messages, so the views themselves now carry that context.
Paths were threaded through the data-handling, experiment, CLI, and
visualization layers as bare strings, so every join was an os.path call or an
f-string that silently depended on a trailing slash. Private helpers now take a
strict Path and documented entry points accept str | Path, normalizing once.
Three string sentinels had to go first, because each one breaks on contact with
Path. "TEMPORARY" no longer compares equal once wrapped, so a Path would have
fallen through and created a literal ./TEMPORARY directory; it is now None,
resolved behind resolve_checkpoint_dir. The empty-string result_path and
out_path relied on os.path.join("", x) returning x, where str(Path("")) gives
".". And landmark's set_state guard accepted only str, dropping a Path without
complaint.
Replacing the checkpoint sentinel exposed a real bug: final_model and the
model_testing runner called model.train directly, without the temporary
directory fallback that training already had, so neither could run without an
explicit checkpoint directory. All three now share checkpoint_dir_or_temporary.
--path_out defaults to "results" instead of "results/", since the plot writers
no longer need the trailing slash to concatenate.
Ray storage paths, the CurveCurator TOML values, the cv-split filename
predicates, and the persisted landmark data_path stay strings; each site says
why.
The loader is generic over omics views, so the gene-expression-specific "ge" name no longer described what the DataFrame contained.
Two of the three fallback directories never existed in any commit, and the package ships no data/, so the search order implied more uncertainty about the gene lists' location than there is. Keep the one dev-checkout fallback and record why it is only reachable from a source tree.
components/data_loading held two unrelated things: the modules that read feature CSVs off disk, and the ones that work out which views a model config asks for. The readers only ever imported drevalpy.datasets, so they sat a layer above the data they consume, and the five cell-line featurizers that need omics had to reach up into components to load it. multiomics.py and views.py now live in datasets/loading, leaving components/data_loading with just the config-aware pair: feature_loaders, which dispatches on a ModelConfig and so needs the registry and models.config, and view_resolution, which maps featurizer names to legacy view strings. datasets does the loading; components decides what to load. Nothing enforced that direction before, though it already held. A policy test now pins it, naming the import forms it rejects so that an unrelated dotted path such as networkx.algorithms.components is not read as a violation. The components.data_loading facade drops its re-exports of the two view loaders, which callers take from datasets.loading instead. The tests that patch load_drug_feature_views still name feature_loaders, since that is where the name is bound.
load_tissue_features and load_cell_line_id_features each forwarded their two arguments to a datasets.feature_tables function and added nothing else. No caller ever reached them: the module's own dispatch calls load_tissues_from_csv and load_cl_ids_from_csv directly, so the wrappers only ever appeared in the facade that re-exported them.
The six thin wrapper functions in view_resolution.py are deleted. ModelConfig gains cell_line_views(), drug_views(), cell_line_entity_id_only(), and drug_entity_id_only(); ResolvedModelConfig gains cell_line_views() and drug_views() that forward to the template with resolved=self. All callers updated; no behaviour change.
No circular dependency exists since hyperparameter_space.py only imports from the standard library.
These methods are the active checkpoint persistence protocol, not legacy bridges.
Every subclass duplicated the same `if batch.response is None` check. Now base `fit()` validates once and delegates to abstract `_fit()`.
Structural invariants (response/pair-index lengths match n_pairs) are now enforced at construction time. This removes the scattered validate_matrix_fit calls and deletes the _matrix_fit module entirely.
The batch now guarantees response is always float64 when present, removing redundant np.asarray(..., dtype=np.float64) casts from every predictor _fit method and from batch construction helpers.
_fit and predict are now @AbstractMethod, matching how MatrixPredictor and FeatureFreePredictor already require subclass implementations. Also updates the doc example to use _fit.
batch.response is already guaranteed float64 by ModelInputBatch.
One-hot values are exact 0/1 integers; floating-point tolerance is unnecessary.
…ictor. Backward compatibility for old single-algorithm payloads is no longer needed; set_state now only accepts the modern per-drug 'algorithms' mapping format.
Each of the 8 literature predictors (SRMF, SparseGO, Precily, PharmaFormer, MOLIR, SuperFELTR, DIPK, DrugGNN) now subclasses BlockPredictor and implements _fit/predict by reading feature blocks from the batch, eliminating the FeatureDataset/LiteratureTrainingMixin bridge layer entirely. Deleted bridge infrastructure: - SingleDrugBlockPredictor, FeatureDatasetBlockPredictor - _algorithm_lifecycle, _block_inputs, _feature_dataset_from_batch - _training_helpers (LiteratureTrainingMixin) - Per-predictor algorithm.py and state.py bridge files
Replace per-predictor custom Dataset classes with a single `make_tensor_loader` helper that converts numpy arrays to a TensorDataset-backed DataLoader. MOLIR and SuperFELTR now pre-expand entity-level omics matrices to pair-level before training, removing all FeatureDataset and RegressionDataset dependencies.
…kage Relocate Predictor, BlockPredictor, MatrixPredictor, and FeatureFreePredictor into drevalpy/components/predictors/abstract/ for clearer separation of the class hierarchy from concrete implementations. Re-exports from __init__.py preserve convenience imports.
lightgbm, xgboost, requests, mygene, and obonet are declared core dependencies — remove try/except ImportError wrappers and import them at module level. Installed the missing packages in the dev venv.
Introduce IndexedPairDataset that stores entity-level feature matrices compactly and performs the entity-to-pair lookup per sample in __getitem__. This eliminates O(n_pairs * d) memory allocations that caused OOM on large drug-response matrices — memory now scales with O(n_entities * d + n_pairs) instead. All 6 torch-based predictors (NeuralNetwork, MOLIR, SuperFELTR, PharmaFormer, Precily, SparseGO) updated to use the new make_pair_loader with entity matrices + pair indices.
Eliminates the module-level _CONSTRUCTED_CACHE dict and _canonical_config_key helper in favour of @functools.cache on _generate_model_class, reducing boilerplate while preserving class-identity stability guarantees. Co-authored-by: Cursor <cursoragent@cursor.com>
The `resolved` parameter was always provided in practice; remove the unused `config` fallback kwarg and make `resolved` non-optional. This tightens the type signatures of the .config and .resolved properties, eliminating None-checks at call sites. Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist wandb, data_loading, and _model_persistence imports to module level since they have no circular dependency on drp_model. The tuning imports must remain deferred due to the DRPModel ↔ tuning cycle. Co-authored-by: Cursor <cursoragent@cursor.com>
Configure uv index routing so users can choose between CPU-only, CUDA 12.6, and CUDA 13.0 PyTorch builds via --extra cpu/cu126/cu130. The Dockerfile defaults to CPU, cutting ~2 GB of nvidia-* packages from the image. GPU builds use --build-arg TORCH_BACKEND=cu126|cu130.
MultiQC v2 requires Section.content to be a string, not None. Sections with only a plot would pass None, causing a pydantic ValidationError.
Implements a new `drevalpy.curation` module that uses the curve_curator Python API directly (no subprocess) to fit 4-parameter log-logistic models to dose-response data. Features chunk-level parallelism via ProcessPoolExecutor for efficient load balancing across dose-range groups, and returns an AnnData with all curve metrics as layers (no QC filtering applied by default). Adds `drevalpy curate` CLI command accepting CSV/Parquet input via UPath.
`_import_modules` caught ImportError/AttributeError and logged only at DEBUG
level, so a component module that fails to import simply vanished from the
registries with no trace. The docs build then failed with a bare count
mismatch ("expected 27 predictors, got 18") that gave no clue about the
underlying import failure.
Skipped modules are now recorded with their full traceback, reported at
WARNING level, and exposed via `get_skipped_builtin_modules()`. The docs
component-catalog validation appends those tracebacks to its RuntimeError so
the real cause is visible in the build log.
Also clean up the curation module added in this PR: add the missing module
docstring, drop the dead `if TYPE_CHECKING: pass` block, apply ruff format,
and split `fit_groups` into helpers to get under the complexity limit.
…h cu13
The docs build failed with a bare catalog count mismatch (18 of 27 predictors,
16 of 17 cell-line featurizers) because `import torch` raised:
ImportError: .../torch/lib/libtorch_cuda.so: undefined symbol: ncclCommResume
nvidia-nccl-cu12 and nvidia-nccl-cu13 both install
`nvidia/nccl/lib/libnccl.so.2`, so whichever wheel is unpacked last owns that
path. On Linux xgboost pulls nvidia-nccl-cu12 while the CUDA 13 torch wheel that
a plain `uv sync` resolves pulls nvidia-nccl-cu13. Adding the cu126 extra made
the resolver select nvidia-nccl-cu12==2.29.3 (down from 2.30.7) for the no-extra
environment too, and 2.29.3 predates the `ncclCommResume` symbol that torch's
cu13 build links against -- verified by inspecting the wheels: the symbol is
present in nvidia-nccl-cu12 2.30.7 and nvidia-nccl-cu13 2.29.7, absent in
nvidia-nccl-cu12 2.29.3.
Overriding nvidia-nccl-cu12 to >=2.30.7 on Linux collapses the two candidate
versions into one that is symbol-compatible with the cu13 torch build, so the
shared library works regardless of unpack order. Verified in an x86-equivalent
Linux container: torch imports, all 17/10/27 components register, no modules
skipped, and `sphinx-build -W` succeeds.
Tighten the n_estimators and max_depth ranges and drop the criterion option to reduce tuning cost, since absolute_error trees are very slow to fit and the extreme ends of the ranges rarely won.
Gene-list resolution searched the download cache and a repo-local data/meta path, neither of which exists in an installed environment, so landmark featurizers crashed in the container pipeline. The lists now live next to the featurizers and are resolved only from there, making gene selection independent of cache and download state.
Every pipeline worker on AWS Batch started with a cold HuggingFace cache, because the Nextflow modules point XDG_CACHE_HOME at the per-task work dir. Hundreds of concurrent tasks therefore pulled the 342 MB checkpoint anonymously through a single NAT gateway, HF replied with 429s, and transformers reported it as a misleading "Can't load the model" OSError. The mirrored snapshot loads bit-identical weights and is now cached per process rather than reloaded for each of the 17 fits in a tuning run. Also drop the hardcoded "orakl" AWS profile, which exists in no environment and made every artifact download raise ProfileNotFound, and repoint the dataset registry at the bucket that actually holds the .h5mu files.
tests/conftest.py acquired its fixtures by adding scripts/ to sys.path and importing download_dataset from it, but scripts/ is not in the repository and data/ is gitignored. The ImportError was swallowed by a bare except that returned False, so a clean checkout produced 1205 FileNotFoundError setup errors instead of one clear failure. Every test was affected, including test_cache_dir.py, which has no data dependency at all: a session-scoped autouse fixture reads the TOY CSVs. The .pre-commit-config.yaml pytest hook runs the full suite, so committing was equally broken on a fresh clone. Nothing in the package ever read those CSVs; they were written and read only by conftest. Fixtures now come from tests/synthetic/, which assembles a 24x8 raw-omics MuData in memory and lets the real featurizers derive fingerprints and drug graphs from real SMILES, so the fixture cannot drift from production. conftest.py drops from 485 to 106 lines and no longer mutates DREVALPY_CACHE_DIR at import time, which is what masked the breakage locally. Featurizers needing pretrained weights are marked network and deselected, as their artifact bucket is not readable without credentials. The suite now passes from a clean export with no data/ and no scripts/. Three library defects surfaced while removing the scaffolding: - Omics reads passed the public view name straight to mdata.mod, so MOLIR, SuperFELTR and the MultiView presets asked for copy_number_variation_gistic while every published .h5mu stores copy_number_variation, and failed for all users. Reads now resolve through OMICS_ACCESSORS, preferring the name as written so both dataset generations work through one path. - PredictorBase.fit filters NaN pairs across the whole batch, but subset_pairs rejected multi-drug masks whenever early stopping was active, which broke nine models. Set membership generalizes it; the single-drug case is the one-element instance of the same rule. - MolGNet multiplied a list by data.num_nodes, which is Optional[int], and ChemBERTa called a tokenizer that its own type allowed to be None. Fixing the ten hasattr sites ty reported also collapsed five copies of the same sparse-densify block into one to_dense helper. ty now reports zero diagnostics, down from 14. Finally, tests mirror the source tree one file per public module, documented in AGENTS.md. Three test directories mirrored packages that no longer exist after the registry consolidation in c76a78b. Uncovered public modules drop from 104 to 76 and are tracked by the existing warn-only mirror policy test rather than by stub files.
The quickstart could not be followed. It invoked flags removed in v2 (--run_id, --dataset_name, --test_mode, --path_out, --baselines), imported load_mudataset and mu_experiment, which are absent from the package, and told readers to install with Poetry although the project builds with uv and hatchling and has no [tool.poetry] section. Every example loaded TOYv1 or TOYv2, which are legacy v1 datasets that the registry does not contain, so they would fail even spelled correctly. Examples now use GDSC1 and the real CLI and Python surfaces, and the dataset tables list exactly the seven registered names. cli/pipeline_commands.rst is deleted rather than rewritten: all 17 stepwise commands it documented were removed in v2, so there was nothing left to correct, and the surviving CLI is generated from the Typer app into cli/_generated_reference.rst. A hand-maintained duplicate is how these pages drifted in the first place. The nextflow blocks in the README kept their parameters, which are real pipeline options rather than drevalpy flags, and now say so. dreval_colab_demo.ipynb is removed; it targeted the pre-v2 API and was one of only two sources of unresolved-import diagnostics. make_gene_lists.ipynb becomes _make_gene_lists.py so the provenance of the shipped gene lists stays readable and lintable. The leading underscore keeps it out of component discovery, which scans package directories for public modules. Its per-omic dataset lists lose the TOY entries rather than substituting one name, since each list enumerates the datasets carrying that omic; CCLE is consequently the only proteomics source. Also correct a documented seven-fold CV default that is five, a drevalpy-report entry point that is not declared, a --wandb-project flag that exists only in the Python tuning API, and a nox session with no noxfile.
The file is Finder metadata with no bearing on the project, but it was committed and unignored, so it reappeared in git status on every macOS checkout and kept getting swept into unrelated commits.
The test suite had no way to notice when a module shipped untested: 76 of 148 public modules had no mirrored test file, and whole user-facing areas were unguarded - the CLI is the primary entry point and had zero tests, `types/results/` was never constructed by any test, and `visualization/` sat at ~21%. Reported coverage also flattered us, because bare `--cov` measured `tests/` itself and silently omitted package files that nothing imported. Scope coverage to the package, mirror the remaining modules, and enforce both in the hook so the gap cannot silently reopen: - `[tool.coverage.run] source = ["drevalpy"]` so never-imported files count as 0% instead of vanishing from the report. - Two-level gate: global `fail_under = 88` plus a per-file floor of 60% via `tools/coverage_gate.py`, whose exemption table records each below-floor module with the reason it cannot comply yet. The gate prints redundant entries so the debt list visibly shrinks. - Wire `pytest --cov-report=json` and the gate into pre-commit and CI, so `--no-verify` cannot bypass the floor. - Close the mirror backlog and flip `test_module_mirror_policy` from warning to failing, accepting the underscore-stripped names AGENTS.md already prescribed. One module is exempt: the omitted maintenance script `_make_gene_lists.py`. Coverage 67% -> 89.5%; 1242 -> 3044 tests in ~57s. Tests pin observed behaviour rather than intended behaviour where the two diverge, so several latent bugs are now documented by passing tests instead of hidden: `SuperFELTRegressor` keeps its encoders in a plain tuple and so breaks on GPU/MPS, `drevalpy/__init__.py` shadows its own submodules and defeats dotted-path monkeypatching, and the pinned curve_curator fork makes `fit_type="MLE"` unreachable. These are left unfixed here to keep the change reviewable.
Closing the test gap turned up six defects that had no coverage to catch them. Each fix comes with the regression test that would have caught it. SuperFELTRegressor kept its encoders in a plain tuple, so Lightning's device placement never reached them: on a GPU/MPS host the regressor moved and the encoders stayed behind. Registering them in an `nn.ModuleList` fixes placement but would also start training pretrained weights and let `trainer.fit`'s `train()` call reactivate their BatchNorm statistics, so the optimizer is narrowed to the regression head, the encoders are frozen, and `train()` is overridden to keep them in eval. The CPU-pinning test fixture that worked around this is gone, which is the real proof. `fit_type="MLE"` was reachable from both `curate()` and `--fit-type` but always died inside the pinned curve_curator fork with `_Model.fit_mle() got an unexpected keyword argument 'weights'`. It is now rejected up front with a message naming that incompatibility instead of failing after preprocessing; the private plumbing still forwards the value, so re-enabling is a one-line change. `single()` handed `split_masks.metadata` to `RunResult` by reference, and `run()` reuses those dicts across models and folds, so mutating a result corrupted its own inputs. It is copied now. `drevalpy/__init__.py` re-exported `run`, `single`, `load`, `randomization` and `robustness` as functions that shadowed the same-named submodules, which broke `import drevalpy.run` and made dotted-path monkeypatching fail in a way that reads as a test bug. The five modules are now private, so the public names are unambiguously the functions. Dropping the submodules also unmasked `drevalpy.experiment` re-documenting `RunResult`, which is owned by `drevalpy.types.results`; it stays importable but leaves `__all__`, keeping the `-W` docs build clean. Also: deleted unreachable guards in `model_input_build.py` that a prior validation call always pre-empted, and made `config_lock()` reentrant via `is_singleton=True` so nesting two blocks no longer deadlocks to the 10s timeout (filelock pinned to >=3.15.2, where singleton init settled).
Replaying 6337 completed Optuna trials from a CTRPv2 benchmark sweep turned up several ranges whose upper halves bought no accuracy: within a predictor the expensive settings scored no better than the cheap ones, so part of every trial budget was being spent on runtime instead of on tuning. Each range is cut back to the region the trials actually rewarded. - chemberta: max_length drops 512 and defaults to 256. Attention cost is quadratic in sequence length and no benchmark SMILES needs 512 tokens. - fingerprints: n_bits drops 4096, which doubled the design matrix for indistinguishable scores. - xgboost: max_depth 12 -> 8, n_estimators 500 -> 300. lightgbm: n_estimators 500 -> 300. - randomForest: max_samples 0.9 -> 0.5. gradientBoosting: max_depth 30 -> 12. - adaboost: n_estimators 200 -> 100, and min_samples_split/min_samples_leaf become non-tunable, having shown neither runtime nor accuracy signal. ElasticNetPredictor declared max_iter, tol and selection in non_tunable_hyperparameters, but _make_estimator never forwarded them, so all three were silently discarded and sklearn's defaults applied instead. The forwarding fix ships here rather than separately because it is what makes the convergence settings reachable at all; the two are not independently meaningful. Ridge stays out of the selection= path, not being coordinate descent. Forwarding then exposed a reproducibility bug that the dropped arguments had been masking: selection="random" with random_state=None takes its coordinate order from unseeded global randomness, so two fits on identical data disagree, which is enough to fail the facade/resolved-config parity test. Both coordinate-descent predictors are seeded with random_state=0 now. Keeping random selection instead of reverting to cyclic is deliberate - sklearn notes it "often leads to significantly faster convergence especially when tol is higher than 1e-4", and tol=1e-2 over features as correlated as ~867 landmark genes plus 2048 fingerprint bits is exactly that regime. LassoPredictor carried the same missing seed independently of the ElasticNet change, so it is fixed alongside; its max_iter also leaves the tunable space, since the binding constraint on convergence was feature scaling rather than the iteration budget.
`response_transformation` never worked. A default of `None` was the only reason nobody noticed, because every way of enabling it either corrupted results or crashed: - Training read raw responses straight from `response_matrix` while predictions were inverse-transformed regardless. With `standard` that multiplied predictions by the training std and added the mean, so the transform degraded output instead of round-tripping. - Nothing in the HPO path ever called `fit`. `single()` passed the unfitted prototype into `hpam_tune` and fitted only its own clone afterwards, so `inverse_transform` raised `NotFittedError` in every Optuna trial. - No CLI flag existed, leaving the pipeline's `response_transformation` parameter with nothing to reach. The contract is symmetric now: fit on the training scope only, transform the training target, inverse-transform predictions, and score against the untouched response matrix so metrics stay in original response units. `fit_response_transformation` is the one place a response transformer is ever fitted, and it clones the prototype so callers can reuse it across folds. The fitted instance reaches training through a keyword argument rather than by rewriting `Dataset.response_matrix`, because ground truth reads that same property -- transforming it in place would have transformed the targets too and silently cancelled the inverse out. Four of the six `_extract_response_pairs` call sites take the transform: both `_ComponentStack` training paths, its early-stopping supervision (training-time targets, so they must share the space), and `DRPModel.train`. The two evaluation sites stay raw. That split is invisible in the metrics when wrong, so tests pin each site. The CLI defaults to `standard`. Affine target scaling is a no-op for squared-error trees and KNN, which the equivalence test asserts, so most of the benchmark is unaffected; gradient-based predictors optimize better on unit-variance targets, and the regularized linear models gain an `alpha` range whose meaning no longer depends on the response scale. Note this shifts the reported baseline for those two families. The docs pages for both entry points described the old behaviour -- the CLI page claimed `single` took the same options as `run`, and the Python page documented the argument as accepting strings rather than a `TransformerMixin` -- so both are corrected here.
Generating a report for 97 models was OOM-killed at 36 GB and again at 60 GB. comparison_scatter retained a full point cloud for every model pair: at 96 models that is 4,560 pairs x 231,080 two-key dicts at 240 B each, about 253 GB, so no container size could have worked. regression_scatter independently retained 5.33 GB of point dicts across 96 models, and wrote all 22.2M of them to multiquc_data. Replace both with bounded aggregations. comparison_scatter now stores a float32 models-by-groups correlation matrix (0.24 MB) and plots per-drug and per-cell-line correlation behind two model dropdowns, restoring the semantics the predecessor implementation documented. regression_scatter becomes a hexbin density image, which is what MultiQC already flattened these plots to above plots_flat_numseries. Retained state is now flat in the number of predictions rather than quadratic in the number of models. Stop loading data no plot reads: HPO trial predictions (2.37 GB) are skipped through with_trials=False, drug_ids and cell_line_ids are interned out of fixed-width <U40/<U9 arrays (4.4 GB to 0.4 GB), and the .h5mu is no longer read at all since every compute() ignores it. Loading drops 10.3x. Fixing the memory alone still produced no report. leaderboard asked for a "Pearson: normalized" metric key that normalize() never emits, so the PCC column was entirely NaN and set_xlim rejected the axis. It passed CI only because the synthetic fixture injected that key unconditionally. Resolve metric names through a shared helper that accepts either spelling, derive axis bounds from finite values while admitting negatives, and warn and skip rather than crash where a metric is absent. heatmap, violin and critical_difference shared the same assumption. Add memory and progress logging, emitted before each plot allocates, so the next failure of this kind names its culprit instead of exiting 137 in silence. Build figures off matplotlib.figure.Figure so pyplot's global registry no longer retains them. Peak RSS on the real 97-model results is 2.62 GB in 4m24s.
A plugin whose import raised was swallowed into a logger.warning, so every
component it would have registered simply vanished, and the absence resurfaced
much later as an "unknown predictor" error far from its cause. Load outcomes are
now recorded and readable through get_failed_plugins() and get_loaded_plugins(),
and DREVALPY_STRICT_PLUGINS=1 re-raises so a plugin's own CI fails on its own
bugs. The default stays non-fatal: one broken third-party package should not
brick the CLI for everyone else.
Plugin authors previously had to reach into five deep module paths that were free
to move under them. drevalpy.plugin now re-exports the 45 symbols an extension
actually needs and drevalpy/py.typed ships the type information that goes with
them, so the internal layout stays free to change while the aliases stay a
promise. drevalpy.testing ships the synthetic dataset and batch builders,
check_plugin and the conformance checks that until now every plugin repo
hand-rolled. docs/python/extensions.rst is rewritten against the real API, and
its examples live as executable modules under docs/examples/ that the docs build
imports and registers, so they cannot rot unnoticed. A new `drevalpy list`
sub-app exposes all five registries plus plugin load status.
Two import-time side effects are gone. critical_difference.py called
matplotlib.use("agg") at module scope, and because importing drevalpy imports
every builtin visualization, merely importing the library disabled inline
plotting in any notebook; matplotlib already falls back to agg without a
display, so the line bought nothing. Separately, the default editable install
put the bare project root on sys.path, so a consumer's `import tests.conftest`
resolved to drevalpy's own tests -- hatch's dev-mode-exact maps only the
drevalpy package, which is why `editables` had to become a real project
dependency rather than a dev-group one.
Breaking changes to the registration API:
- Registration now rejects classes with unimplemented abstract methods rather
than accepting them and failing at first use.
- A class-body `contract` (or `cell_line_contract` / `drug_contract`) is a valid
declaration where it previously raised TypeError. The decorator argument still
wins when both are given.
- Registering an already-registered splitter mode raises instead of silently
overwriting it, with override=True for the cases where replacement is meant;
visualization registration, which already raised, gained the same escape
hatch.
- visualization.table() returns a DataFrame instead of a str.
- metadata() was added to the splitter and visualization registries so all five
can be introspected the same way.
Component validation also moved ahead of first use, so a malformed component is
rejected at registration. fail_under moves 88 -> 89 now that measured coverage
sits at 90.1%.
The pre-commit hook ran the whole suite with coverage: 98.6s warm, 281.5s cold. Measuring it showed the cost was not the 3830 tests - 3397 of them contribute ~0.0s between them - but the ~30 that spawn a fresh interpreter, each paying `import drevalpy`. That import cost 3.59s, because `drevalpy/registry/__init__.py` registers builtins eagerly and every registered predictor pulled its training stack in at module scope. Move the heavy imports into the functions that need them (16 libraries, including pytorch_lightning, torch_geometric, torch, sklearn and mudata), keeping registration eager so a fresh process still discovers all 27 predictors and 27 featurizers. `import drevalpy` is now 0.21s and the CLI starts in 0.24s, which every user pays on every invocation, not just tests. tests/test_import_cost_policy.py locks this in; two shapes it guards do not have the "move the import into the method" remedy, so they are documented: a forbidden base class must move module, and a module-scope side effect must stay eager - deferring the xgboost thread defaults segfaults the suite once torch's OpenMP is loaded first. Then split the suite. `uv run pytest` now defaults to the fast tier via addopts and takes 15s for 98% of tests; the 65 tests that spawn interpreters, fit curves or train models carry `slow` and run in CI. A test earns the mark by costing >=0.2s measured as wall time saved when its whole fixture-sharing group is deselected - not by summed per-test durations, which now mislead because caching hoisted the cost into shared setup. Coverage and the per-module gate move to CI on the full suite, where the floors are meaningful; a green hook no longer says anything about coverage, and AGENTS.md says so. Share the fixtures that were rebuilt per test: one wheel build, one fitted curve set, one report. No tests were deleted; the suite grew by 79 and coverage went 90% to 91.13%. Three latent bugs surfaced, each invisible only because serial collection order hid it: - tests/visualization/test_base.py faked IPython without `version_info`, which matplotlib probes once per process when the first canvas is created. It passed only when an earlier test had already tripped the probe. - test_spec.py, test_extensions.py, test_validation.py and test_block_specs.py leaked registrations into the global registries. The last three called register_builtin_components() as teardown, which only adds and never evicts. test_extensions.py was saved by two unrelated tests' finally blocks. pytest-xdist is deliberately not adopted: with the import cost gone it gains at most ~11% on the CI suite and is slower on the fast tier. It earned its keep as a diagnostic - it is what shook the order dependencies out.
A `pca[gene_expression]:identity:lightgbm` run in the container died with
`OSError: libgomp.so.1: cannot open shared object file`. LightGBM's wheel is
the only one in the dependency set that links the system OpenMP by bare
soname: `lib_lightgbm.so` carries `NEEDED libgomp.so.1` with no RPATH and no
vendored copy, while the xgboost and scikit-learn wheels ship an
auditwheel-renamed `libgomp-e985bcbb.so.1.0.0` and find it through their own
RPATH. The runtime stage is `python:3.13-slim-bookworm`, which has no
`libgomp1`, and only `/opt/venv` is copied out of the full-bookworm builder,
so the builder's copy never reaches the image.
It had never been satisfied properly, only by accident. torch bundles
`torch/lib/libgomp.so.1` with the *unrenamed* soname and loads it RTLD_GLOBAL
through `libtorch_global_deps.so`, so once torch was imported the loader found
`libgomp.so.1` already in the link map and never touched the filesystem. Every
predictor used to pull its training stack in at module scope, so `import
drevalpy` imported torch and LightGBM worked as a side effect. Deferring those
imports removed the accident, and a lightgbm-only run - which never imports
torch - was the first to notice. An `ldd` sweep over every `.so` in the image
confirms `lib_lightgbm.so` is the only file left with an unresolved
`libgomp.so.1`, so `libgomp1` in the runtime stage is the whole fix; reverting
the deferral is not, since tests/test_import_cost_policy.py exists to keep
torch out of `sys.modules`.
The build now imports each native stack in a separate process. Separate
processes are the point: a single combined import passes on an image missing
libgomp1, because importing torch first resolves LightGBM's dependency for it,
which is exactly how this stayed hidden. A missing system library fails
`docker build` instead of surfacing 200 lines into a Nextflow log.
That log is the second half of this commit. All 16 Optuna trials failed with
this OSError, and the run still reported `Best hyperparameters: {...}` before
dying later in `model.train()`, because a raising trial was logged and turned
into NaN and `_resolve_best_params` then fell back to defaults. A trial that
runs and scores non-finite is an inconclusive tuning result and defaults are
the right answer there; a trial that raises is a fault, and if every one of
them raised there is nothing to fall back from. `hpam_tune` now raises
`HPOTrialsFailedError` in that case, chaining the first exception, so the
OSError heads the traceback instead of being 200 lines of scrollback away from
the line that reported success. A study where only some trials raised still
tunes, and warns with the counts. Trials keep returning NaN into the study, so
a search space with a few invalid corners behaves as before.
`_optuna_objective` in hpo_runtime.py has the same swallow but is left alone:
`build_optuna_objective` has no shipped caller, and a second copy of this
logic would be worse than the duplication is now.
Verified in the rebuilt image: lightgbm imports and fits with torch absent
from `sys.modules`, `import drevalpy` still leaves torch unimported, and the
failing model spec runs through `single()` with HPO to tuned hyperparameters
rather than defaults. Hiding `libgomp.so.1` again reproduces the original
fault as `HPOTrialsFailedError` with the `OSError` as `__cause__`, no
`Best hyperparameters` line and exit 1.
The packaged registry gains the seven orakl_v2 screens plus TOYv1, which broke two tests that asserted against a literal seven-name list and a literal count. Derive both from available_datasets.json instead, so the next dataset addition does not need a test edit, and assert the invariant that actually matters: builtin_datasets and builtin_sources mirror the packaged JSON exactly. The original seven stay behind as an explicit presence guard so an accidental deletion still fails.
The refit screens ship every fitted curve, and they store pEC50 as the response matrix. A pEC50 exists for every curve that converged, so `~isnan(response_matrix)` counted all 384 pairs of TOYv1 as usable observations - including the 163 the fit scores as noise. Every splitter was training and evaluating on them. `curve_quality_mask` exposes each CurveCurator quality metric the datasets carry as a keyword option, and the four built-in splitters now blank the failing pairs before splitting. Called bare it applies `relevance_score >= -log10(0.05)` and `abs(fold_change) >= 0.45`, the alpha and fc_lim this repo already passes to CurveCurator; on TOYv1 that reproduces CurveCurator's own `regulation != 0` label with zero mismatches across all 384 pairs. Gating on relevance_score rather than p_value is the load-bearing choice: p_value is the raw, uncorrected F-test value, and with mtc_method = "sam" the multiple-testing control lives entirely in relevance_score. Filtering on p_value across the ~60k curves in a screen would correct for nothing. It stays available, off by default, along with every other metric, so a plugin can filter on anything the dataset holds. A table drives the checks so fifteen options do not mean fifteen branches, and the tests parametrize over it - a new rule cannot land without a boundary and an off-by-default case. Consequences worth flagging: - The layers are guaranteed by the file format, so there is no capability check and no silent fallback; a missing layer raises. That means the older generation of the screens, which spells the metrics in CamelCase and has no `regulation`, can no longer be loaded, and the registry now points the plain dataset names at the refit files. CCLE is dropped until its refit exists in the bucket, which also empties the proteomics gene list - the tuples are updated and the shipped CSVs left alone. - The refit reused the old file names, so a cache from an older drevalpy holds a file that parses but has no quality layers. `load` served it forever and it then failed deep inside a split, so a file that cannot be quality-filtered is now treated as stale and fetched again. - `MuDataLike` gains `response_layer_names` and `get_response_layer`. Breaking for anyone implementing the protocol by hand; documented.
Two independent defects in the curve fitting, both silent. CurveCurator computes a standard error for every fitted parameter on every fit at every speed, from a Moore-Penrose pseudo-inverse of the Jacobian, and emits pEC50 Error alongside the slope, front and back errors. All four were dropped on the floor because _COLUMN_RENAME did not list them, so the pipeline shipped a million curves with no uncertainty estimate at all - the one number that says how much to trust a pEC50. They are now renamed and carried through as layers, pec50_error next to the X it quantifies. Normalization ran inside each parallel chunk, and curve_curator derives its factors from the median over the rows of whatever frame it is handed. With cores=8 a normalized dataset therefore got eight independent sets of factors, which made its output a function of the core count - reproducible only by accident. _normalize now computes the factors once per dose-range group before chunking and disables curve_curator's own normalization, so the split cannot be observed. Signal Quality needs care here: run_pipeline derives it from the raw control intensities that normalization overwrites, so it is computed before the overwrite, carried on the frame and restored after the fit. tests/curation/test_normalize.py compares two core counts byte for byte. Also names the fit speeds, defaults them through DEFAULT_FIT_SPEED, and rejects an unknown speed up front rather than letting curve_curator fail mid-fit.
The response matrix holds pEC50 - build_anndata pivots that column into X - but the leaderboard subtitled every plot "LN_IC50", so a reader comparing runs was told the wrong measure. Higher pEC50 is more potent while lower LN_IC50 is, so the label inverted the direction of the axis it described. ALLOWED_MEASURES went with it. Nothing in drevalpy imported it; its only reference was a test asserting the list was well-formed, which made it look like a validated contract while validating nothing. A measure name is checked where it is used - get_response_layer raises on a missing layer and names the ones that exist - so the list was a second, unenforced source of truth. Also lifts the CSV/Parquet read in the curate command into a helper and records in both docstrings why the .h5ad is the intermediate the curation pipeline wants: curate keys obs_names/var_names from the label columns it was handed, so a caller can fit on native identifiers and remap later without refitting.
The dependency list had drifted in both directions: entries nothing imports, and imports nothing declares. Seven declarations had zero import sites. `starlette` was the clearest case - it was added as a `>=0.49.1` floor three days after CVE-2025-62727 to push a *docs-only* transitive (sphinx-autobuild's live-reload server) past the advisory, but it landed in [project.dependencies], so every install carried an ASGI framework. Also gone: `mygene` and `obonet`, with no reference anywhere; `toml`, because both drevalpy and curve_curator read TOML with stdlib `tomllib`; `typing-extensions` and `importlib-resources`, both redundant at `>=3.11`. `flaky` moves to the dev group, where its only user (tests/test_evaluation.py) lives, and `pygments`/`types-toml` leave it - the latter stubbed the package above. `fsspec[full]` becomes `fsspec[s3,http]`. Only `s3://` (the default artifacts URI), `http(s)://` and `file://` (fsspec core) are reachable from shipped code, but `full` is 18 backends, so the lock carried dask, distributed, panel, paramiko, pygit2, dropbox, adlfs, the google-cloud and grpcio stacks, ocifs and smbprotocol to serve protocols nothing here can reach. Registering another protocol now means installing its backend, which fsspec reports by name. Three imports were undeclared and worked only by accident. `anndata` is the curation pipeline's return type and is imported at module scope in `curation/_anndata.py`; `joblib` is imported at module scope in `_persistence_io.py`, so model save/load would have broken had scikit-learn dropped it; `pyarrow` is the engine behind `cli/curate.py`'s `pd.read_parquet`, and reached us only via `fsspec[full]` - narrowing that extra without declaring it would have broken `drevalpy curate input.parquet`. All three are now explicit. `torch-scatter` was the most expensive dependency per line of use: a compiled extension with no wheels for current torch, needing the `extra-build-dependencies` escape hatch to build at all, for a single `scatter(..., reduce="add")` in MolGNet's hand-rolled message-passing base. torch_geometric has vendored an equivalent `scatter` since 2.3 and no longer requires torch-scatter, and the file already imported from `torch_geometric.utils`. The project now compiles nothing from source: a clean install with no pre-existing torch went from a build-isolation failure to 5.6s. Verified rather than assumed: embeddings from the real 300 MB MolGNet.pt, over five molecules spanning 3-15 atoms, are `np.array_equal` before and after with torch_scatter uninstalled. Network tests pass unchanged (7 passed, 1 skipped - the skip is a ragged-varm limitation, not auth), the full `-m "not network"` suite is 4125 passed / 9 skipped, molgnet still registers with `get_skipped_builtin_modules()` empty, and all 10 prek hooks pass. uv.lock drops 363 -> 302 packages.
`.repowise/`, `.vscode/` and `.mcp.json` are per-machine: an MCP server list, editor extension recommendations and a knowledge-graph cache. They carry no meaning for another checkout and were only showing up as noise in `git status`.
Repowise had wired Claude Code, VS Code and Cursor from one init, though Cursor is the only host installed here. Unwiring the other two removed .mcp.json, so its ignore entry named a file that no longer exists, while the .cursor/ config it did leave behind was unignored and embeds an absolute path to this checkout. .vscode/ stays: repowise rewrites it on every update regardless of the vscode_mcp opt-out.
The file had grown into a narrative of how the test suite got fast: exact timings, benchmark tables, and the war stories behind each guard. Those numbers go stale on the next commit and buried the rules that still apply, so collapse each story to the rule it implies, gather every command into one block at the top, and promote the import-cost guard out of a nested bullet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains the current draft of v2.0.0 of drevalpy. It comes with the following changes:
What is currently missing: