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

[TheiaProk] Add additional input enabling characterization #547

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

sage-wright
Copy link
Member

@sage-wright sage-wright commented Jul 17, 2024

This PR closes #545.

πŸ—‘οΈ This dev branch should be deleted after merging to main.

🧠 Aim, Context and Functionality

User requested ability to skip characterization and just perform assembly & assembly QC. This change adds the perform_characterization boolean which defaults to true. Users can set it to false to skip all genomic characterization

πŸ› οΈ 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 : Yes/No

All TheiaProk workflows

πŸ“‹ Workflow/Task Step Changes

  • all organism specific tasks are thrown into the if (perform_characterization) { boolean switch.

πŸ”„ Data Processing

Docker/software or software versions changed:

Databases or database versions changed:

Data processing/commands changed:

File processing changed:

Compute resources changed:

➑️ Inputs

  • Boolean perform_characterization = true has been add to all TheiaProk workflows

⬅️ Outputs

  • None. If perform_characterization = false then only read QC, assembly, and assembly QC will be performed

πŸ§ͺ Testing

Test Dataset

PE success

Commandline Testing with MiniWDL or Cromwell (optional)

Terra Testing

Suggested Scenarios for Reviewer to Test

Theiagen Version Release Testing (optional)

πŸ”¬ Final Developer Checklist

  • The workflow/task has been tested locally and results, including file contents, are as anticipated
  • The workflow/task has been tested on Terra and results, including file contents, are as anticipated
  • The CI/CD has been adjusted and tests are passing (to be completed by Theiagen developer)
  • Code changes follow the style guide

🎯 Reviewer Checklist

  • All impacted workflows/tasks have been tested on Terra with a different dataset than used for development
  • All reviewer-suggested scenarios have been tested and any additional
  • All changed results have been confirmed to be accurate
  • All workflows/tasks impacted by change/s have been tested using a standard validation dataset to ensure no unintended change of functionality
  • All code adheres to the style guide
  • MD5 sums have been updated
  • The PR author has addressed all comments

πŸ—‚οΈ Associated Documentation (to be completed by Theiagen developer)

  • Relevant documentation on the Public Health Resources "PHB Main" has been updated
  • Workflow diagrams have been updated to reflect changes

@sage-wright sage-wright marked this pull request as ready for review July 18, 2024 12:29
@kapsakcj kapsakcj self-requested a review July 23, 2024 20:31
Copy link
Contributor

@kapsakcj kapsakcj left a comment

Choose a reason for hiding this comment

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

Nice work, I reviewed the code and everything looks good (just hard to see diff because of indentation). I have no requested changes.

I made a new data table named PR547_test_sample for testing these changes. I launched a couple of samples per input data type (ILMN PE, SE, ONT, FASTA) with characterization on and off.

workflows launched:

@kapsakcj
Copy link
Contributor

Looks like documentation needs to be updated as I don't see the new optional input in the TheiaProk inputs table - could you please do that when you are able? I'll merge the PR once that is completed.

Not sure if we want to update the workflow diagram or not, might be useful to help highlight this new feature to users.

@kapsakcj kapsakcj merged commit ba1804b into main Jul 24, 2024
10 checks passed
@kapsakcj kapsakcj deleted the smw-optional-characterization-dev branch July 24, 2024 20:01
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.

[TheiaProk] add optional input to skip characterization
2 participants