theiagen / public_health_bioinformatics

Bioinformatics workflows for genomic characterization, submission preparation, and genomic epidemiology of pathogens of public health concern.
GNU General Public License v3.0
33 stars 15 forks source link

pangolin_update wf bug fix and hiding one input param related to Flu and organism_parameters subwf #415

Closed kapsakcj closed 2 months ago

kapsakcj commented 2 months ago

This PR closes #.

🗑️ This dev branch should be deleted after merging to main.

:brain: Aim, Context and Functionality

  1. Pangolin_Update workflow: We missed deleting the nextclade_dataset_reference_input from org param call block when updating to nextclade V3 in #375 . This caused an error in Terra since the workflow was non-functional and did not pass miniwdl check or womtools validate

  2. Additionally, for theiacov_ONT and TheiaCoV_Illumina_PE workflows: added gene_locations_bed_file to org param call blocks to hide optional input from user. This input should be hidden from the user in Terra because it will not be used even if a File is provided by the user.

:hammer_and_wrench: Impacted Workflows/Tasks & Changes Being Made

This will affect the behavior of the workflow(s) even if users don’t change any workflow inputs relative to the last version : No

Running this workflow on different occasions could result in different results, e.g. due to use of a live database, "latest" docker image, or stochastic data processing : No

:clipboard: Workflow/Task Step Changes

🔄 Data Processing

Nothing has changed, these are just workflow level Input changes

Docker/software or software versions changed: No

Databases or database versions changed: No

Data processing/commands changed: No

File processing changed: No

Compute resources changed: No

➡️ Inputs

  1. for theiacov_ONT and TheiaCoV_Illumina_PE workflows: added gene_locations_bed_file to org param call blocks to hide optional input from user
  2. nextclade_dataset_reference_input from org param call block for Pangolin_Update workflow

⬅️ Outputs

N/A

:test_tube: Testing

Test Dataset

Pangolin_Update test: 4 sars-cov-2 samples

TheiaCoV_Illumina_PE and ONT: Not sure? Perhaps test with Flu samples through both?

Commandline Testing with MiniWDL or Cromwell (optional)

N/A

Terra Testing

  1. Pangolin_Update test: https://app.terra.bio/#workspaces/theiagen-validations/curtis-sandbox-theiagen-validations/job_history/8ef17805-b8d8-4094-9018-401cf7b53e7e

workflow ran successfully: image

  1. TheiaCoV_Illumina_PE: https://app.terra.bio/#workspaces/theiagen-validations/PHB_Validation_nextcladeV3testing/job_history/521e50b0-db79-4396-b401-6a350def0a0b ⚠️ Just need to check that the workflows ran successfully Workflows ran successfully ✅
  2. TheiaCoV_ONT: https://app.terra.bio/#workspaces/theiagen-validations/PHB_Validation_nextcladeV3testing/job_history/723f8545-f8df-4eb8-9166-c829bfcc19fd ⚠️ Just need to check that the workflows ran successfully Workflows ran successfully ✅

For both of these workflows ⬆️ , the gene_locations_bed_file input has been hidden from the user, as expected: image

Suggested Scenarios for Reviewer to Test

For pangolin_update, any sars-cov-2 samples that have been previously run through Pangolin (via TheiaCoV workflows)

Likely test Flu samples through ILMN PE and ONT workflows

Theiagen Version Release Testing (optional)

:microscope: Final Developer Checklist

🎯 Reviewer Checklist

🗂️ Associated Documentation (to be completed by Theiagen developer)

cimendes commented 2 months ago

Testing flu here: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/12c8a4c9-aea8-47e0-a74f-5cc230066d82

✅ All good!!

cimendes commented 2 months ago

Managed to import this branch without issue on Terra (failed with version in main) for Pangolin_Update_PHB

cimendes commented 2 months ago

Testing pango update here: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/7c2b9d62-1df0-40c6-b000-7fc649aeb1ad

✅ working like a charm!