Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Generalize mutect2 wrapper #607

Draft
wants to merge 1 commit into
base: scatter-gather
Choose a base branch
from

Conversation

tedil
Copy link
Member

@tedil tedil commented Mar 4, 2025

This PR generalizes / combines the mutect2/run and mutect2/prepare_panel wrappers, such that the code is the same but logic can be controlled via snakemake inputs and params (e.g. whether to call tumor/normal or normal only).

TODO

  • config → params treatment
  • adapt calling code of mutect2/run and mutect2/prepare_panel

@tedil tedil self-assigned this Mar 4, 2025
@tedil tedil added the enhancement New feature or request label Mar 4, 2025
@tedil tedil marked this pull request as draft March 4, 2025 09:53
@ericblanc20 ericblanc20 marked this pull request as ready for review March 4, 2025 10:16
@ericblanc20 ericblanc20 marked this pull request as draft March 4, 2025 10:17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: the java_options are taken from the config at the moment. But shouldn't they be taken from the snakemake resources? As far as I understand, you eventually want to put all resources allocation under functions in __init__.py code (which may read the configuration before passing the final resources to snakemake, of course). Is that the plan?

And another question/remark: I notice that you explicitly assign each argument/parameter to a separate value (for example

if germline_resource := config.get("germline_resource"):
    germline_resource_param = f'--germline-resource {germline_resource}'
else:
    germline_resource_param = ""

I like it, the code is nice and clean. I haven't done that in recent branches, based on the fact that I was concerned by the explosion of parameters in some wrappers. Should I reconsider?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java_options is something we have to consider "globally" for snappy, not only here. I think they could potentially be derived from the resources (as for example Xmx is constrained by the memory resource) as params.

As for the separate values: I mainly do this for optional parameters. A utility function such as build_parameter(args, arg_name, cmd_option) would make it a little bit shorter/less repetitive, e.g. optional_parameter(args, "germline_resource", "--germline-resource"). (Perhaps with a better name.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants