relax requirement for map file and fix function call#118
Conversation
Makes the map file optional and calculates genetic positions from the provided recombination rate if no map is provided. Also fix make_constant_recombination_from_pgen call related to PalamaraLab#117
There was a problem hiding this comment.
Pull request overview
This PR relaxes the requirement for a genetic map file, making it optional. When no map file is provided, genetic positions are calculated from physical positions using a constant recombination rate. The PR also fixes a bug where make_constant_recombination_from_pgen was being called with an incorrect number of arguments.
Key changes:
- Made the
--mapoption optional in the CLI - Fixed function call to
make_constant_recombination_from_pgenby removing the erroneouschromparameter - Minor whitespace cleanup (trailing spaces removed)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/threads_arg/main.py | Removed required=True from --map option and cleaned up trailing whitespace |
| src/threads_arg/infer.py | Fixed make_constant_recombination_from_pgen function call by removing incorrect chrom parameter and cleaned up trailing whitespace |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
hey paolo, this is really great, thanks I don't think the chromosome verification block you added is appropriate within that if-clause, the region/chromosome should be checked against the pgen variants for a wider set of circumstances and I think that's out of scope for your PR (more generally we should only be accepting single-chromosome pgen files) if you remove that block I'll be happy to merge and I'll open up an issue/separate PR with pgen/region verification |
|
Dear @ArniFreyrG , I don't understand very well your comments. Maybe you are referring to a comment that Copilot AI has made to the file: this is only a suggestion that AI made but should be considered carefully, the code isn't modified by AI yet. I've just resolved the conversation with AI for clarity. The only changes I made are small changes that can be seen on Files changed (2) tab. Please, can you confirm me if that those changes work for you? you can also start a revision and select the code I need to drop out. Many thanks for your support |
|
My bad, mistook the copilot comment for actual edits. I've just started builds/tests and I'll approve+merge if/when everything passes |
relax requirement for map file and fix function call
Makes the map file optional and calculates genetic positions from the provided recombination rate if no map is provided.
Also fix make_constant_recombination_from_pgen call
related to #117