Code Cleanup

[dfornika]

• Remove any unnecessary commented-out code
• Standardize variable names
• Collect related variables into collections (dictionaries) eg. collect file paths into a paths dict.
• Document functions
• Improve documentation in README.md
• Review consistency of USAGE messages in all job scripts.

[DiDigsDNA]

Let's check that variables listed in USAGE statement match those actually parsed in job scripts.

[dfornika]

Good idea. I've added it to the list above.

[DiDigsDNA]

USAGE statement variables in bwa_mem.sh script don't match those referenced later in script.

cpo-pipeline/data/job_scripts/bwa_mem.sh

Line 11 in 6277800

USAGE="qsub $( basename "$BASH_SOURCE" ) [-h] -f|--fasta\n\

 case $key in
    -1|--R1)
    # input_R1.fastq.gz file
    input_r1_fastq="$2"
    shift # past argument
    shift # past value
    ;;
    -2|--R2)
    # input_R2.fastq.gz file
    input_r2_fastq="$2"
    shift # past argument
    shift # past value
    ;;
    -r|--reference)
    # reference fasta file
    reference="$2"
    shift # past argument
    shift # past value
    ;;
    -o|--output)
    # output sam file
    output="$2"
    shift # past argument
    shift # past value
    ;;
  esac
done

I can change the USAGE string variables.

[DiDigsDNA]

In this case, the USAGE statement contains the correct variables used in the script but these variables don't get initialized to "" (whereas an unused variable does).

cpo-pipeline/data/job_scripts/fastqc.sh

Line 22 in 6277800

input_fastq=""

input_fastq="" This variable not used in script.
These variables are used but aren't initialized:
input_r1_fastq="$2"
input_r2_fastq="$2"

I can fix this.

[DiDigsDNA]

USAGE statement includes all variables used in script. However, the alignment variable is initialized to "", while the model variable isn't.

cpo-pipeline/data/job_scripts/iqtree.sh

Line 20 in 6277800

alignment=""

Do you want me to add model="" to line 21?

[DiDigsDNA]

USAGE stmt has all the variables used within the script and 3 of 4 variables used in the script are initialized. I suspect treefile="" at line 21 should be replaced with output_file=""... do you agree?

cpo-pipeline/data/job_scripts/maskrc-svg.sh

Line 20 in 6277800

alignment=""

[DiDigsDNA]

1. 2 of 3 variables initialized to "" except for label --> would you like me to change this?

   cpo-pipeline/data/job_scripts/mlst.sh

   Line 22 in 6277800

   input=""

2. Comment should read # input contigs.fasta file

   cpo-pipeline/data/job_scripts/mlst.sh

   Line 31 in 6277800

   # input_R1.fastq.gz file

3. It looks like input variable was changed to assembly:
   

   cpo-pipeline/data/job_scripts/mlst.sh

   Line 32 in 6277800

   assembly="$2"

   
   

   cpo-pipeline/data/job_scripts/mlst.sh

   Line 55 in 6277800

   mlst --label "${label}" "${assembly}" > "${output_file}"

   
   ...so I'll replace input="" with assembly="".

[DiDigsDNA]

I noticed 2 issues with samtools_filter_fixmate_sort.sh:

1. Confusing comment. I don't understand the use of the term integer here.

   cpo-pipeline/data/job_scripts/samtools_filter_fixmate_sort.sh

   Line 36 in 6277800

   # only include reads with none of the FLAGS in this integer present

2. Shouldn't this comment read # output sam (or bam) file ?

   cpo-pipeline/data/job_scripts/samtools_filter_fixmate_sort.sh

   Line 42 in 6277800

   # only include reads with none of the FLAGS in this integer present

[DiDigsDNA]

Inconsistency in variables initialized and those used:

cpo-pipeline/data/job_scripts/samtools_fixmate.sh

Line 23 in 6277800

flags=0

And the output variable not initialized:

cpo-pipeline/data/job_scripts/samtools_fixmate.sh

Line 38 in 6277800

output="$2"

I suggest changing initial variable declarations to:

input=""
output=""

...and deleting flags=0 (because the flags variable isn't used)
...and changing

cpo-pipeline/data/job_scripts/samtools_fixmate.sh

Line 37 in 6277800

# only include reads with none of the FLAGS in this integer present

to read
`# output sam (or bam) file

[DiDigsDNA]

Two issues with this script:

1. Need to add -n|--name-order to USAGE stmt.

   cpo-pipeline/data/job_scripts/samtools_sort.sh

   Line 11 in 6277800

   USAGE="qsub $( basename "$BASH_SOURCE" ) [-h] -i|--input SAM_BAM -o|--output SAM_BAM\n\

2. Should this comment read # output sam (or bam) file ?

   cpo-pipeline/data/job_scripts/samtools_sort.sh

   Line 43 in 6277800

   # only include reads with none of the FLAGS in this integer present

[DiDigsDNA]

queries is initialized and declared but not reference otherwise in script:

cpo-pipeline/data/job_scripts/seqtk_totalbp.sh

Line 24 in 6277800

queries=""

I can delete it.

[DiDigsDNA]

I noticed 4 issues with shovill.sh:

1. -q|--queries declared in USAGE stmt but not used in script --> suggest removing it

   cpo-pipeline/data/job_scripts/shovill.sh

   Line 11 in 6277800

   USAGE="qsub $( basename "$BASH_SOURCE" ) [-h] -1|--R1 INPUT_R1_FASTQ -2|--R2 INPUT_R2_FASTQ -q|--queries QUERIES_MSH -o|--output_file OUTPUT_FILE\n\

2. -o|--output_file in in USAGE stmt doesn't match below, where -o|--output_dir is parsed

   cpo-pipeline/data/job_scripts/shovill.sh

   Line 55 in 6277800

   -o|--output_dir)

3. -c|--mincov used in script but not declared or initialized and not mentioned in USAGE stmt. --> Should this be initialized to "" or some default value?

   cpo-pipeline/data/job_scripts/shovill.sh

   Line 45 in 6277800

   mincov="$2"

4. -l|--minlen used in script but not declared or initialized and not mentioned in USAGE stmt. --> Should this be initialized to "" or some default value?

   cpo-pipeline/data/job_scripts/shovill.sh

   Line 49 in 6277800

   -l|--minlen)

[DiDigsDNA]

2 issues with snippy_ctgs.sh:

1. 2 unused variables --> suggest eliminating them

   cpo-pipeline/data/job_scripts/snippy_ctgs.sh

   Line 21 in 6277800

   input_r1_fastq=""

   
   

   cpo-pipeline/data/job_scripts/snippy_ctgs.sh

   Line 22 in 6277800

   input_r2_fastq=""

2. Suggest adding ctgs="" declaration to line 21 as this variable assigned later

   cpo-pipeline/data/job_scripts/snippy_ctgs.sh

   Line 38 in 6277800

   ctgs="$2"

   
   Note that snippy perl code sets default for ctgs to '' so this should work.
3. USAGE stmt needs to instruct the user to specify a contigs file on the command line
   i.e. instead of USAGE="qsub $( basename "$BASH_SOURCE" ) [-h] -r|--ref REFERENCE_FASTA -c|--ctgs -o|--outdir OUTPUT_FILE
   use this:
   USAGE="qsub $( basename "$BASH_SOURCE" ) [-h] -r|--ref REFERENCE_FASTA -c|--ctgs CONTIGS_FASTA_FILE -o|--outdir OUTPUT_FILE

[DiDigsDNA]

Only thing I noticed for snp-dists.sh is that comment states that the alignment file is a reference fasta

cpo-pipeline/data/job_scripts/snp-dists.sh

Line 29 in 6277800

# reference fasta file

I suggest changing this to read # alignment fasta

[DiDigsDNA]

Suggestion for snp-sites.sh to change comment from

cpo-pipeline/data/job_scripts/snp-sites.sh

Line 29 in 6277800

# reference fasta file

to read # alignment fasta