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: use filtered variants in tmb and signatures #470

Merged
merged 8 commits into from
Dec 20, 2023

Conversation

ericblanc20
Copy link
Contributor

No description provided.

@ericblanc20 ericblanc20 linked an issue Nov 22, 2023 that may be closed by this pull request
@ericblanc20 ericblanc20 requested a review from mbenary November 22, 2023 01:04
@coveralls
Copy link

coveralls commented Nov 22, 2023

Coverage Status

coverage: 85.882% (-0.03%) from 85.915%
when pulling e536bdb on 469-use-filtered-variants-in-tmb-and-signatures
into 50d0781 on main.

Copy link

  • Please format your Python code with black: make black
  • Please format your Snakemake code with snakefmt: make snakefmt
  • Please organize your imports isorts: make isort
  • Please ensure that your code passes flake8: make flake8

You can trigger all lints locally by running make lint

@ericblanc20 ericblanc20 changed the title 469 use filtered variants in tmb and signatures feat: use filtered variants in tmb and signatures Nov 23, 2023
Copy link
Contributor

@mbenary mbenary left a comment

Choose a reason for hiding this comment

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

LGTM

config = self.w_config["step_config"]["somatic_variant_signatures"]
name_pattern = "{mapper}.{caller}"
if config["is_filtered"]:
if len(config["filters"]) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

tiny detail: in all cases beforehand the if-clause is the other way around starting with if len(config["filters"]) == 0 . Meaning is always the same, so the phrase could also be the same.

threshold = config.get("ebfilter_threshold", 0)
elif "eb_filter" in config and "ebfilter_threshold" in config["eb_filter"]:
threshold = config["eb_filter"].get("ebfilter_threshold", 0)
elif "ebfilter" in config and "ebfilter_threshold" in config["ebfilter"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intended to allow "ebfilter" as well as "eb_filter"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, I believe I understood. It's "eb_filter" vs "one_ebfilter", correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think both might be used in the code.

eb_filter vs one_ebfilter: it needed while we keep the old way of filtration (dkfz+eb_filter+oxo_g). one_ebfilter is the new way of filtering.

@ericblanc20 ericblanc20 merged commit 3e3cead into main Dec 20, 2023
8 checks passed
@ericblanc20 ericblanc20 deleted the 469-use-filtered-variants-in-tmb-and-signatures branch December 20, 2023 17:01
@tedil tedil mentioned this pull request Jun 28, 2024
This was referenced Dec 9, 2024
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.

use filtered variants in tmb and signatures
3 participants