theiagen / public_health_bacterial_genomics

GNU Affero General Public License v3.0
26 stars 14 forks source link

add agrvate to S. aureus subwf pull request #218

Closed kapsakcj closed 1 year ago

kapsakcj commented 1 year ago

Opened a separate branch to ease testing on Terra without disrupting anyone currently using the cjk-saureus-subwf branch.

This PR adds the tool agrvate to the merlin magic subworkflow for any samples identified as "Staphyloccus aureus" by GAMBIT.

Setting as a draft until I'm done testing in Terra Testing in Terra is complete, I'm comfortable merging this into the cjk-saureus-subwf dev branch.

Tested both TheiaProk_Illumina_SE: https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/curtis_sandbox/job_history/058e88f0-df97-4094-a7e7-08a312d959a2

and TheiaProk_Illumina_PE (failures here are due to test data, not code changes): https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/curtis_sandbox/job_history/83b43378-ae76-488d-a70f-5f0b96ec9df2

Changes

    File agrvate_summary = summary TSV
    File agrvate_results = tarball of all agrvate output files
    String agrvate_agr_group = final agr group desitnation
    String agrvate_agr_match_score = confidence in agr match, higher number the better. ranges 0-15. 15 is max
    String agrvate_agr_canonical = canonical or non-canonical agrD gene
    String agrvate_agr_multiple = number of agr groups found (expect only 1 per isolate)
    String agrvate_agr_num_frameshifts = number of frameshifts found in CDS of extracted agr operon (done during mummer step)
    String agrvate_version = version
    String agrvate_docker = docker image used

I have NOT adjusted CI, so after merging I expect to see some CI failures on the other PR. Currently the CI is setup to only run on pull requests to main branch and not to others, so we can fix the CI stuff on PR #213

kapsakcj commented 1 year ago

@cimendes when you get a chance can you please give the code a look over & test in Terra on some samples? I would like to get this PR merged before we merge PR #213

cimendes commented 1 year ago

I tested the branch on 50 S. aureus successfully on terra (https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/0c754c08-5955-49aa-83c5-2bae53ade521, one failed due to an unrelated issue), and obtained the expected output columns. :) LGTM! I think we can go ahead and merge this one.

image

kapsakcj commented 1 year ago

merge away! Thank you for testing!

cimendes commented 1 year ago

@kapsakcj can I delete the branch?