Add --LR_model and --demo options to gene_vec_model.py#3
Closed
chronicgiardia wants to merge 2 commits into
Closed
Add --LR_model and --demo options to gene_vec_model.py#3chronicgiardia wants to merge 2 commits into
chronicgiardia wants to merge 2 commits into
Conversation
Refactor gene_vec_model.py to expose the embedding pipeline via main() and add helpers for the new CLI arguments: - --LR_model: train (or load) a logistic regression model over gene-pair embedding features and persist it as gene_pair_lr.pkl in the directory. - --demo: export a copy of the learned embeddings into the demo directory. Add a unittest suite covering argument parsing, feature construction, LR model save/load, and demo export. Document both arguments in the README. Co-Authored-By: Oz <oz-agent@warp.dev>
Author
|
Superseded by #4, which contains only the --LR_model/--demo changes on a dedicated branch off main (excludes the unrelated Pylint workflow commit). |
There was a problem hiding this comment.
Pull request overview
This PR extends the gene_vec_model.py training script with two optional CLI switches to support downstream usage: training/loading a gene-pair logistic regression model and exporting learned embeddings into a demo directory. It also refactors the script into an import-safe main() entrypoint and adds unit tests for the new helper functions.
Changes:
- Added
--LR_model(train/load persisted LR model) and--demo(export embeddings for demo consumption) tosrc/gene_vec_model.py, extracting helper functions for testability. - Added
src/test_gene_vec_model.py(stdlibunittest) to cover argument parsing, feature construction, LR save/load behavior, and demo export. - Documented the new options in
README.mdand introduced a Pylint workflow.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/test_gene_vec_model.py |
Adds unittest coverage for the new CLI-backed helper functions and LR/demo behaviors. |
src/gene_vec_model.py |
Adds --LR_model / --demo, factors the training pipeline into main(), and introduces helper functions for LR training/loading and demo export. |
README.md |
Documents the new optional CLI arguments and resulting outputs. |
.github/workflows/pylint.yml |
Adds a GitHub Actions workflow intended to run Pylint checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11
to
+23
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v3 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
| - name: Install dependencies | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| pip install pylint | ||
| - name: Analysing the code with pylint | ||
| run: | | ||
| pylint $(git ls-files '*.py') |
| file inside ``lr_dir``; if that file already exists it is loaded instead | ||
| of being retrained. Returns a tuple ``(model, model_path)``. | ||
| ''' | ||
| from sklearn.linear_model import LogisticRegression |
Comment on lines
+136
to
+141
| sample_size = len(left_input) | ||
| smaple_idx = np.arange(sample_size) | ||
| np.random.shuffle(smaple_idx) | ||
| left_input = left_input[smaple_idx] | ||
| right_input = right_input[smaple_idx] | ||
| targets = targets[smaple_idx] |
Comment on lines
+151
to
+157
| fw = open(outfile, 'w') | ||
| for i in range(len(encoded_genes)): | ||
| fw.write(geneids[i]) | ||
| for j in range(len(encoded_genes[0])): | ||
| fw.write(',' + str(encoded_genes[i, j])) | ||
| fw.write('\n') | ||
| fw.close() |
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
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.
Summary
Adds two optional CLI arguments to
src/gene_vec_model.pyand documents them:--LR_model <dir>: after training gene embeddings, trains (or loads, if already present) a logistic regression model that predicts whether a pair of genes is associated, using the element-wise product of the two genes' embeddings as features. Persisted asgene_pair_lr.pklin the directory. Requires scikit-learn.--demo <dir>: exports a copy of the learned embeddings into the demo directory (into itsdata/subdirectory when present) so the demo scripts can consume freshly trained embeddings.To make this testable, the training pipeline was refactored into a
main()guarded byif __name__ == '__main__', and the new logic was extracted into small helpers (build_pair_features,train_or_load_lr_model,export_embeddings_for_demo). CLI behavior is unchanged.Tests
Adds
src/test_gene_vec_model.py(stdlibunittest, no pytest required) covering argument parsing, feature construction, LR model save/load (including load-without-retrain), and demo export. Run with:All 11 tests pass.
Notes
This branch also includes a pre-existing fork commit, "Add Pylint workflow for Python code analysis" (
6a4b10f), which was already on the fork'smain.Conversation: https://app.warp.dev/conversation/2180fe28-77cf-4cc1-9147-26b5fd7a2e6a