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

Conversation

sofstam
Copy link
Contributor

@sofstam sofstam commented May 2, 2024

Description

Changed

  • According to previous communication, cust053 would like to have data and analysis even though they are just below the targeted threshold of 75%. In case the data are below the targeted threshold, they will assess if the quality of the data is good enough for the purposes. In this PR, the function use_read_count_threshold is updated to fit this purpose.

How to prepare for test

  • Ssh to relevant server (depending on type of change)
  • Use stage: us
  • Paxa the environment: paxa
  • Install on stage (example for Hasta):
    bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_cg -t cg -b read_count_threshold -a

How to test

  • Do ...

Expected test outcome

  • Check that ...
  • Take a screenshot and attach or copy/paste the output.

Review

  • Tests executed by
  • "Merge and deploy" approved by
    Thanks for filling in who performed the code review and the test!

This version is a

  • MAJOR - when you make incompatible API changes
  • MINOR - when you add functionality in a backwards compatible manner
  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

  • Document in ...
  • Deploy this branch on ...
  • Inform to ...

@sofstam sofstam requested a review from a team as a code owner May 2, 2024 14:46
Copy link

sonarqubecloud bot commented May 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@@ -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.

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.

3 participants