Skip to content

feat: make the gene list of single-view sklearn models configurable - #464

Open
jfrog64 wants to merge 2 commits into
daisybio:developmentfrom
jfrog64:feat-sklearn-gene-list-hyperparameter
Open

feat: make the gene list of single-view sklearn models configurable#464
jfrog64 wants to merge 2 commits into
daisybio:developmentfrom
jfrog64:feat-sklearn-gene-list-hyperparameter

Conversation

@jfrog64

@jfrog64 jfrog64 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

PR Checklist for all PRs

  • This comment contains a description of changes (with reason)
  • Referenced issue is linked — no existing issue, came up while benchmarking baselines on other gene spaces
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated — no new module and no signature that the API docs render, so nothing to add

Changes

New features

load_single_cell_line_view hard-codes the gene list it subsets gene_expression with:

if "gene_expression" in cell_line_views:
    return load_and_select_gene_features(
        feature_type="gene_expression",
        gene_list="landmark_genes_reduced",
        ...
    )

So every single-view sklearn baseline — ElasticNet, RandomForest, SVR, SingleDrug*, … — is
locked to the landmark genes. The other gene lists that ship with the framework
(drug_target_genes_all_drugs, gene_list=None for the full matrix) are reachable for hand-written
models, but not for the baselines, even though the baselines are exactly where one wants to vary the
feature space to see how much of a model's performance comes from the gene selection rather than the
learner.

This makes it configurable in two steps, without changing any default:

  1. load_single_cell_line_view gains gene_list: str | None = "landmark_genes_reduced" and forwards it.
    The default is the previously hard-coded value, so every existing caller behaves exactly as before.

  2. SklearnModel gains a gene_list class attribute with the same default, and build_model reads it
    from the hyperparameters:

    self.gene_list = hyperparameters.get("gene_list", type(self).gene_list)

    This is the same pattern the class already uses for cell_line_views and drug_views one line above.
    The value stays in self.hyperparameters, so save()/load() carry it and predict() uses the same
    gene space the model was trained on.

gene_list is deliberately not added to baselines/hyperparameters.yaml, so the tuning grids are
untouched and nothing changes for existing runs. It becomes available to anyone who passes a custom
hyperparameter set.

Tests: tests/test_sklearn_gene_list.py builds a temporary meta/gene_lists directory with two lists
and covers

  • omitting gene_list reproduces the previously hard-coded landmark_genes_reduced behaviour
  • an explicit list is honoured
  • gene_list=None loads the full expression matrix
  • the SklearnModel default equals the class attribute
  • the "gene_list" hyperparameter reaches load_cell_line_features and lands in self.hyperparameters
  • setting it on one instance does not leak into the class attribute or into other instances

Bug fixes

Maintenance

jfrog64 and others added 2 commits August 26, 2026 16:38
load_single_cell_line_view hard-coded landmark_genes_reduced for
gene_expression, so the sklearn baselines could not be run on a different
gene space. It now takes a gene_list argument that defaults to the previous
value, and SklearnModel exposes it as the "gene_list" hyperparameter. The
value is kept in self.hyperparameters so save()/load() carry it and predict()
uses the same gene space the model was trained on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MODEL_FACTORY is typed as returning DRPModel, which has no gene_list, so mypy
rejected the attribute accesses in the test. Instantiate ElasticNetModel
directly and assert once that MODEL_FACTORY["ElasticNet"] really is that class,
which keeps the registration covered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.10%. Comparing base (7d24cd6) to head (3269d9f).
⚠️ Report is 75 commits behind head on development.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #464      +/-   ##
===============================================
+ Coverage        80.34%   82.10%   +1.75%     
===============================================
  Files              101      121      +20     
  Lines             8171     9833    +1662     
===============================================
+ Hits              6565     8073    +1508     
- Misses            1606     1760     +154     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants