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

[Standardization] Standardizing the language behind percent identity and coverage #751

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

awh082834
Copy link
Contributor

@awh082834 awh082834 commented Feb 6, 2025

This PR closes #557 and #558

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

🧠 Summary

Still in testing and hunting down any other variations of these inputs. Opening this now for discussion and input on any that I may have missed!

Standardizing the language of percent identity and coverage to min_percent_identity and min_coverage wherever possible.
Inputs that use decimal values for identity have been kept to min_identity to thwart confusion between percent and decimal values.

⚡ Impacted Workflows/Tasks

Tasks:

  • emmtyper
  • pbptyper
  • abricate_vibrio
  • abricate
  • pasty
  • kleborate
  • lissero
  • virulencefinder
  • abricate_abaum
  • kaptive
  • ectyper
  • amrfinder
  • resfinder
  • plasmidfinder
  • ts_mlst
  • tbprofiler
  • srst2_vibrio

Workflows:

  • TheiaProk
  • TheiaCov
  • merlin_magic

This PR may lead to different results in pre-existing outputs: No

This PR uses an element that could cause duplicate runs to have different results: No

🛠️ Changes

Instances of mincov, minident, or any variation of percent identity and coverage have been changed to min_percent_identity and min_coverage.

⚙️ Algorithm

No algorithmic changes.

➡️ Inputs

abricate_flu_minid -> abricate_flu_min_percent_identity
kaptive_min_identity -> kaptive_min_percent_identity
ectyper_opid -> ectyper_o_min_percent_identity
ectyper_hpid -> ectyper_h_min_percent_identity
virulencefinder_identity_threshold -> virulencefinder_min_percent_identity
kleborate_min_identity -> kleborate_min_percent_identity
lissero_min_id -> lissero_min_percent_identity
pasty_min_pident -> pasty_min_percent_identity
emmtyper_percid -> emmtyper_min_percent_identity
pbptyper_min_pident -> pbptyper_min_percent_identity
abricate_vibrio_minid -> abricate_vibrio_min_percent_identity
abricate_flu_minid -> abricate_flu_min_percent_identity
abricate_abaum_minid -> abricate_abaum_min_percent_identity
abricate_mincov -> abricate_min_coverage
abricate_flu_mincov -> abricate_flu_minc_coverage
ectyper_opcov -> ectyper_o_min_coverage
ectyper_hpcov -> ectyper_h_min_coverage
virulencefinder_coverage_threshold -> virulencefinder_min_coverage
lissero_min_cov -> lissero_min_coverage
tbp_parser_coverage_threshold -> tbp_parser_min_coverage
abricate_vibrio_mincov -> abricate_vibrio_min_coverage
srst2_vibrio_min_cov -> srst2_vibrio_min_coverage
amrfinderplus_mincov -> amrfinder_plus_min_coverage
amrfinderplus_minid -> amrfinder_plus_min_identity (not percent identity as the input is not a percentage. Done to prevent confusion.)
resfinder_min_cov -> resfinder_min_coverage
plasmidfinder_min_cov -> plasmid_finder_min_coverage
ts_mlst_minid -> ts_mlst_min_percent_identity
ts_mlst_mincov -> ts_mlst_min_coverage

⬅️ Outputs

No outputs have been changed.

🧪 Testing

TheiaCov:

ONT
FASTA
Illumina_PE
SE not tested as it does not contain wf_flu_track which utilizes task_abricate.

TheiaProk:

ONT
FASTA
Illumina_PE
Illumina_SE

Suggested Scenarios for Reviewer to Test

🔬 Final Developer Checklist

  • The workflow/task has been tested and results, including file contents, are as anticipated
  • The CI/CD has been adjusted and tests are passing (Theiagen developers)
  • Code changes follow the style guide
  • Documentation and/or workflow diagrams have been updated if applicable
    • You have updated the "Last Known Changes" field for any affected workflows in the respective workflow documentation page and for every entry in the three workflows_overview tables to be the tag for the next upcoming release. If you do not know the tag, please put "vX.X.X"

🎯 Reviewer Checklist

  • All changed results have been confirmed
  • You have tested the PR appropriately (see the testing guide for more information)
  • All code adheres to the style guide
  • MD5 sums have been updated
  • The PR author has addressed all comments
  • The documentation has been updated

@awh082834 awh082834 marked this pull request as ready for review February 10, 2025 20:45
@awh082834 awh082834 requested a review from a team as a code owner February 10, 2025 20:45
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.

standardize how we say percent identity
1 participant