Skip to content

Conversation

@G-kodes
Copy link

@G-kodes G-kodes commented Jul 19, 2021

Description

An implementation of the GATK ValidateVariants tool

QC

For all wrappers added by this PR, I made sure that

  • there is a test case which covers any introduced changes,
  • input: and output: file paths in the resulting rule can be changed arbitrarily,
  • rule names in the test case are in snake_case and somehow tell what the rule is about or match the tools purpose or name (e.g., map_reads for a step that maps reads),
  • all environment.yaml specifications follow the respective best practices,
  • wherever possible, command line arguments are inferred and set automatically (e.g. based on file extensions in input: or output:),
  • all fields of the example rules in the Snakefiles and their entries are explained via comments (input:/output:/params: etc.),
  • stderr and/or stdout are logged correctly (log:), depending on the wrapped tool,
  • temporary files are either written to a unique hidden folder in the working directory, or (better) stored where the Python function tempfile.gettempdir() points to (see here; this also means that using any Python tempfile default behavior works),
  • the meta.yaml contains a link to the documentation of the respective tool or command,
  • Snakefiles pass the linting (snakemake --lint),
  • Snakefiles are formatted with snakefmt,
  • Python wrapper scripts are formatted with black.

Lord have mercy on my code. PR's are stressful lol

log:
"results/sample_VALID.log",
params:
R="genome.fasta",
Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot find this to be used in the wrapper. On the other hand, extra is missing. Maybe you meant to write

Suggested change
R="genome.fasta",
extra="", # optional extra arguments

input:
vcf="sample.vcf",
output:
"results/sample_VALID.vcf",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the output should just have a .txt suffix or so, since this tool will not create a vcf file, right?

@fgvieira
Copy link
Collaborator

fgvieira commented Feb 8, 2022

Maybe it would also be nice to set tmpdir with tempfile.

@johanneskoester johanneskoester added the stale: contribution welcome Please feel free to finalize this work. label Aug 16, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2024

This PR was marked as stale because it has been open for 6 months with no activity.

@github-actions github-actions bot added the Stale label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale: contribution welcome Please feel free to finalize this work. Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants