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

Update use_read_count_threshold for taxprofiler #3196

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cg/meta/workflow/taxprofiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,9 @@ def get_multiqc_search_patterns(self, case_id: str) -> dict:
def get_genome_build(self, case_id: str) -> str:
"""Return the reference genome build version of a Taxprofiler analysis."""
return GenomeVersion.T2T_CHM13.value

@property
def use_read_count_threshold(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this method is used somewhere, seems like it's just defined in the nextflow analysis api but it's never called 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

the parent method was removed there https://github.com/Clinical-Genomics/cg/pull/2886/files
so it has to be adapted on the nf-analysis class or has already be done and can just be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove those methods and test start-available for all pipelines (rare disease, tomte, taxprofiler, rnafusion)

Copy link
Contributor

Choose a reason for hiding this comment

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

not the test. Only the methods. I'm not sure if the behaviour is correct right now. Can you check?

Copy link
Contributor Author

@sofstam sofstam May 3, 2024

Choose a reason for hiding this comment

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

def get_sequencing_quality_check_for_case(case: Case) -> Callable:
"""Return the appropriate sequencing quality checks for the workflow for a case."""
workflow: Workflow = case.data_analysis
case_passes_workflows = [
Workflow.BALSAMIC,
Workflow.BALSAMIC_PON,
Workflow.BALSAMIC_QC,
Workflow.BALSAMIC_UMI,
Workflow.MIP_DNA,
Workflow.MIP_RNA,
Workflow.RAREDISEASE,
Workflow.RNAFUSION,
Workflow.TOMTE,
]
any_sample_in_case_has_reads_workflows = [
Workflow.FLUFFY,
Workflow.FASTQ,
Workflow.MICROSALT,
Workflow.MUTANT,
Workflow.TAXPROFILER,
]
if workflow in case_passes_workflows:
return SequencingQCCheck.CASE_PASSES
elif workflow in any_sample_in_case_has_reads_workflows:
return SequencingQCCheck.ANY_SAMPLE_IN_CASE_HAS_READS
raise ValueError(f"Workflow {workflow} does not have a sequencing quality check.")

It is correct, can test as well.

"""Defines whether the threshold for adequate read count should be passed for all samples
when determining if the analysis for a case should be automatically started."""
return False
Loading