[TheiaProk] Enable plasmidfinder to be skipped

[sage-wright]

This PR closes #280, closes #370 and closes #359.

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

:brain: Aim, Context and Functionality

This PR does three things:

1. Provides a user input for users to turn off plasmidfinder
2. Hides the preemptible input parameter for BaseSpace_Fetch (no testing required except for checking the inputs of the workflow on Terra)
3. Disables call caching on RASUSA

: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 : Yes (call caching off on RASUSA)

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 : Yes (RASUSA is random, but we only want to test to see if call caching is actually off

• volatile: true is added to the RASUSA meta field
• the new input parameter call_plasmidfinder (default=true) is added to all TheiaProk workflows
• preemptible is now hard-coded in the BaseSpace_Fetch task to the default preemptible value of 1

:clipboard: Workflow/Task Step Changes

🔄 Data Processing

No 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: - preemptible is now hard-coded to 1 in BaseSpace_Fetch - volatile:true turns off call caching in RASUSA #### ➡️ Inputs

TheiaProk workflows now have the call_plasmidfinder optional workflow input. True by default, no change will be noticed by users of TheiaProk unless they set the value of that variable to false.

⬅️ Outputs

:test_tube: Testing

Test Dataset

Commandline Testing with MiniWDL or Cromwell (optional)

Terra Testing

• [x] preemptible is no longer an input in BaseSpace_Fetch when compared to v1.3.0
• [x] RASUSA did not use call caching (view initial run here and running it again to test call-cache off test here
• [ ] PlasmidFinder turned on and off
  • [ ] TheiaProk_Illumina_PE here
  • [ ] TheiaProk_Illumina_SE here
  • [ ] TheiaProk_ONT here
  • [x] TheiaProk_FASTA here

Suggested Scenarios for Reviewer to Test

Theiagen Version Release Testing (optional)

• New plasmidfinder input variable field with various set to true & false will need to be added to the validation table

:microscope: Final Developer Checklist

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

🎯 Reviewer Checklist

• [x] All impacted workflows/tasks have been tested on Terra with a different dataset than used for development
• [x] 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
• [x] All code adheres to the style guide
• [x] 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
• [x] ~Workflow diagrams have been updated to reflect changes~ NA

[cimendes]

Code changes look good! Testing here:

• BaseSpace_Fetch preemptible no longer exposed ✅

  • 

• Rasusa

  • Sort of unrelated to this PR but genome length is now set to String instead of Int. Can we fix this here?
  • 

• Disabling PlasmidFinder ✅

  • 
  • https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/bf4550da-d8bd-477c-82a7-96e6be72dd19

[sage-wright]

As per the tool documentation, genome length can be either an integer or have a metric suffix, so I think we should keep it as a String

[cimendes]

Given the general context of PHB, we always set the genome length as int so I think it would be easier to document and transmit to our users the same standard! What do you think?

[sage-wright]

I think that warrants further discussion! @kapsakcj and @jrotieno, any opinions?

[kapsakcj]

I would prefer to keep it as a String because of what Rasusa can accept (and the usage is bit more flexible this way)

Plus we have documentation surrounding the use of strings like 5m for this particular workflow

And there are users who are now accustomed to providing these strings as input (all of the labs that have done LOD studies)

EDIT: I agree this does make TheiaProk_ONT input parameters differ from the other TheiaProk workflows.... hmmmmm 🤔

EDIT2: I think we should keep as is, where in RASUSA_PHB standalone workflow the genome_size input is a String. And in TheiaProk_ONT_PHB, the genome_size input is kept as an Int (which is then fed into the read_qc_ont subworkflow and passed into Rasusa task)

[jrotieno]

On one hand I am thinking String is versatile to allow both a string and integer input, while on the other I am not sure that we want to encourage people to input 5Kb instead of 5000 even if the tool allows it.

[cimendes]

I guess this settles it! It shall remain as is! :D Approving the PR and merging!