From 4ebbc9a2de99803efdcb3268e5cc769d693360b6 Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 15:29:55 -0700
Subject: [PATCH 1/7] Update README.md

---
 README.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/README.md b/README.md
index e446e7a..9d192e3 100644
--- a/README.md
+++ b/README.md
@@ -1,5 +1,8 @@
 # MMETSP
 
+This is a review branch.
+
+
 [![Binder](http://mybinder.org/badge.svg)](http://mybinder.org/repo/dib-lab/MMETSP)
 
 This is a work-in-progress repository, automating the khmer protocols over a large-scale RNAseq data set:

From ee570d7f25e1c0cd3fac5747d26199a4fe894648 Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 15:32:53 -0700
Subject: [PATCH 2/7] Update getdata.py

---
 getdata.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/getdata.py b/getdata.py
index 95fabf3..e11ee52 100644
--- a/getdata.py
+++ b/getdata.py
@@ -1,3 +1,5 @@
+# review comments
+
 import os
 import os.path
 from os.path import basename

From 658448f1cca8e53f870bac3c93397c3c1274e701 Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 16:08:57 -0700
Subject: [PATCH 3/7] Update clusterfunc.py

---
 clusterfunc.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/clusterfunc.py b/clusterfunc.py
index 87c1f06..315cf44 100644
--- a/clusterfunc.py
+++ b/clusterfunc.py
@@ -6,6 +6,12 @@
 import subprocess
 from subprocess import Popen, PIPE
 
+DO_QSUB = False
+
+def set_DO_QSUB(val):
+    global DO_QSUB
+    DO_QSUB = val
+    print "DO_QSUB is set to:",DO_QSUB
 
 def check_dir(dirname):
     if os.path.isdir(dirname) == False:
@@ -29,6 +35,7 @@ def get_module_load_list(module_name_list):
 
 
 def qsub_file(basedir, process_name, module_name_list, filename, process_string):
+    global DO_QSUB
     working_dir = os.getcwd()
     qsub_dir, qsub_filename = get_qsub_filename(
         basedir, process_name, filename)
@@ -62,6 +69,7 @@ def qsub_file(basedir, process_name, module_name_list, filename, process_string)
             "env | grep PBS # Print out values of the current jobs PBS environment variables\n")
     qsub_string = 'qsub -V ' + qsub_filename
     print qsub_string
-    s = subprocess.Popen(qsub_string, shell=True)
-    s.wait()
+    if DO_QSUB:
+        s = subprocess.Popen(qsub_string, shell=True)
+        s.wait()
     os.chdir(working_dir)

From 3b5fa66082ac39b7c6bddebaa7684254402b3fc7 Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 16:17:45 -0700
Subject: [PATCH 4/7] Update getdata.py

---
 getdata.py | 46 ++++++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/getdata.py b/getdata.py
index e11ee52..bba78a6 100644
--- a/getdata.py
+++ b/getdata.py
@@ -12,11 +12,16 @@
 import glob
 # custom Lisa module
 import clusterfunc
+# in argparse, execute:
+# clusterfunc.set_DO_QSUB(True) when debug flag is set
 
 # 1. Get data from spreadsheet
 
+# in other files, use this as a module, then use "from getdata import get_data"
+# consider changing your naming conventions
 
 def get_data(thefile):
+    # use csv module in the for loop
     count = 0
     url_data = {}
     with open(thefile, "rU") as inputfile:
@@ -32,15 +37,10 @@ def get_data(thefile):
             ftp = line_data[position_ftp]
             name_read_tuple = (name, read_type)
             print name_read_tuple
-            # check to see if Scientific Name and run exist
-            if name_read_tuple in url_data.keys():
-                # check to see if ftp exists
-                if ftp in url_data[name_read_tuple]:
-                    print "url already exists:", ftp
-                else:
-                    url_data[name_read_tuple].append(ftp)
-            else:
-                url_data[name_read_tuple] = [ftp]
+            # check if name_read_tuple exist, if not then add empty set and add ftp
+            value = url_data.get(name_read_tuple, set())
+            value.add(ftp)
+            url_data[name_read_tuple] = value
         return url_data
 
 # 2. Download data
@@ -68,6 +68,12 @@ def sra_extract(newdir, filename):
     #    print sra_string
     # elif seqtype=="paired":
         # check whether .fastq exists in directory
+    
+    
+    # never use this type of concatenation with path names
+    # use glob.glob(os.path.join(newdir,"*.fastq))
+    # predict filename more specifically
+    # check md5sum?
     if glob.glob(newdir + "*.fastq"):
         print "SRA has already been extracted", filename
     else:
@@ -89,7 +95,10 @@ def fastqc_report(fastq_file_list, newdir, fastqcdir, filename):
         print "fastqc already complete:", filename
     else:
         # creates command to generate fastqc reports from all files in list
+        
+        # find out if this is necessary
         file_string = str(fastq_file_list)
+        
     # print fastq_file_list
         file_string = " ".join(fastq_file_list)
     # print file_string
@@ -145,10 +154,15 @@ def fastqc(newdir, fastqcdir, filename):
             fastq_file_list.append(newdir + i)
     fastqc_report(fastq_file_list, newdir, fastqcdir, filename)
 
-datafile = "SraRunInfo.csv"
-basedir = "/mnt/scratch/ljcohen/mmetsp/"
-clusterfunc.check_dir(basedir)
-for datafile in datafiles:
-    url_data = get_data(datafile)
-    print url_data
-    execute(basedir, url_data)
+def main():
+    datafile = "SraRunInfo.csv"
+    basedir = "/mnt/scratch/ljcohen/mmetsp/"
+    clusterfunc.check_dir(basedir)
+    for datafile in datafiles:
+      url_data = get_data(datafile)
+      print url_data
+      execute(basedir, url_data)
+
+print __name__
+#if __name__ == "__main__":      
+#    main()

From cada52e4fadccb12439cfe6d8ddc022188c90494 Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 16:20:59 -0700
Subject: [PATCH 5/7] Update getdata.py

---
 getdata.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/getdata.py b/getdata.py
index bba78a6..1dcfa5f 100644
--- a/getdata.py
+++ b/getdata.py
@@ -163,6 +163,5 @@ def main():
       print url_data
       execute(basedir, url_data)
 
-print __name__
-#if __name__ == "__main__":      
-#    main()
+if __name__ == "__main__":      
+    main()

From ab0d8eea00b6806b1173d38a5fbee4adf4707c43 Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 16:23:40 -0700
Subject: [PATCH 6/7] Update getdata.py

---
 getdata.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/getdata.py b/getdata.py
index 1dcfa5f..0d884b5 100644
--- a/getdata.py
+++ b/getdata.py
@@ -155,6 +155,7 @@ def fastqc(newdir, fastqcdir, filename):
     fastqc_report(fastq_file_list, newdir, fastqcdir, filename)
 
 def main():
+    clusterfunc.set_DO_QSUB(False)
     datafile = "SraRunInfo.csv"
     basedir = "/mnt/scratch/ljcohen/mmetsp/"
     clusterfunc.check_dir(basedir)

From 00915441ed5e950feda9fdbcc1cdc9ab89bba98a Mon Sep 17 00:00:00 2001
From: Lisa Cohen <ljcohen@ucdavis.edu>
Date: Mon, 12 Sep 2016 23:43:30 -0700
Subject: [PATCH 7/7] Update clusterfunc.py

---
 clusterfunc.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/clusterfunc.py b/clusterfunc.py
index 315cf44..6850249 100644
--- a/clusterfunc.py
+++ b/clusterfunc.py
@@ -35,6 +35,13 @@ def get_module_load_list(module_name_list):
 
 
 def qsub_file(basedir, process_name, module_name_list, filename, process_string):
+    # separate this function into several:
+    # make qsub file
+    # have different argument for system, e.g. PBS, SGE, SLURM, etc.
+    # submit qsub
+    # what if AWS? and no qsub?
+    # suggestion to end each function with a string
+    # rather than execute each command as a batch script
     global DO_QSUB
     working_dir = os.getcwd()
     qsub_dir, qsub_filename = get_qsub_filename(