details: http://www.bx.psu.edu/hg/galaxy/rev/9efe896dbb17 changeset: 3505:9efe896dbb17 user: Kelly Vincent kpvincent@bx.psu.edu date: Wed Mar 10 11:51:00 2010 -0500 description: Fix try-except-finally bug to be compatible with Python 2.4 in several tools and twilltestcase.py. Updated liftOver test to replace hard-coded (incorrect) path and modified code file to allow for new path. Improved bowtie error handling.

diffstat:

test/base/twilltestcase.py | 70 ++++++++++++++------------ tools/extract/liftOver_wrapper.py | 17 ++++-- tools/extract/liftOver_wrapper.xml | 9 +-- tools/extract/liftOver_wrapper_code.py | 6 +- tools/samtools/sam_pileup.py | 44 ++++++++-------- tools/sr_mapping/bowtie_wrapper.py | 72 ++++++++++++++++++--------- tools/sr_mapping/bwa_wrapper.py | 88 +++++++++++++++++---------------- 7 files changed, 171 insertions(+), 135 deletions(-)

diffs (452 lines):

diff -r f84112d155c0 -r 9efe896dbb17 test/base/twilltestcase.py --- a/test/base/twilltestcase.py Wed Mar 10 11:28:50 2010 -0500 +++ b/test/base/twilltestcase.py Wed Mar 10 11:51:00 2010 -0500 @@ -633,24 +633,26 @@ data = self.last_page() file( temp_name, 'wb' ).write(data) try: - if attributes is None: - attributes = {} - compare = attributes.get( 'compare', 'diff' ) - extra_files = attributes.get( 'extra_files', None ) - if compare == 'diff': - self.files_diff( local_name, temp_name, attributes=attributes ) - elif compare == 're_match': - self.files_re_match( local_name, temp_name, attributes=attributes ) - elif compare == 're_match_multiline': - self.files_re_match_multiline( local_name, temp_name, attributes=attributes ) - else: - raise Exception, 'Unimplemented Compare type: %s' % compare - if extra_files: - self.verify_extra_files_content( extra_files, elem.get( 'id' ) ) - except AssertionError, err: - errmsg = 'History item %s different than expected, difference (using %s):\n' % ( hid, compare ) - errmsg += str( err ) - raise AssertionError( errmsg ) + # have to nest try-except in try-finally to handle 2.4 + try: + if attributes is None: + attributes = {} + compare = attributes.get( 'compare', 'diff' ) + extra_files = attributes.get( 'extra_files', None ) + if compare == 'diff': + self.files_diff( local_name, temp_name, attributes=attributes ) + elif compare == 're_match': + self.files_re_match( local_name, temp_name, attributes=attributes ) + elif compare == 're_match_multiline': + self.files_re_match_multiline( local_name, temp_name, attributes=attributes ) + else: + raise Exception, 'Unimplemented Compare type: %s' % compare + if extra_files: + self.verify_extra_files_content( extra_files, elem.get( 'id' ) ) + except AssertionError, err: + errmsg = 'History item %s different than expected, difference (using %s):\n' % ( hid, compare ) + errmsg += str( err ) + raise AssertionError( errmsg ) finally: os.remove( temp_name )

@@ -676,21 +678,23 @@ data = self.last_page() file( temp_name, 'wb' ).write( data ) try: - if attributes is None: - attributes = {} - compare = attributes.get( 'compare', 'diff' ) - if compare == 'diff': - self.files_diff( local_name, temp_name, attributes=attributes ) - elif compare == 're_match': - self.files_re_match( local_name, temp_name, attributes=attributes ) - elif compare == 're_match_multiline': - self.files_re_match_multiline( local_name, temp_name, attributes=attributes ) - else: - raise Exception, 'Unimplemented Compare type: %s' % compare - except AssertionError, err: - errmsg = 'Composite file (%s) of History item %s different than expected, difference (using %s):\n' % ( base_name, hda_id, compare ) - errmsg += str( err ) - raise AssertionError( errmsg ) + # have to nest try-except in try-finally to handle 2.4 + try: + if attributes is None: + attributes = {} + compare = attributes.get( 'compare', 'diff' ) + if compare == 'diff': + self.files_diff( local_name, temp_name, attributes=attributes ) + elif compare == 're_match': + self.files_re_match( local_name, temp_name, attributes=attributes ) + elif compare == 're_match_multiline': + self.files_re_match_multiline( local_name, temp_name, attributes=attributes ) + else: + raise Exception, 'Unimplemented Compare type: %s' % compare + except AssertionError, err: + errmsg = 'Composite file (%s) of History item %s different than expected, difference (using %s):\n' % ( base_name, hda_id, compare ) + errmsg += str( err ) + raise AssertionError( errmsg ) finally: os.remove( temp_name )

diff -r f84112d155c0 -r 9efe896dbb17 tools/extract/liftOver_wrapper.py --- a/tools/extract/liftOver_wrapper.py Wed Mar 10 11:28:50 2010 -0500 +++ b/tools/extract/liftOver_wrapper.py Wed Mar 10 11:51:00 2010 -0500 @@ -4,7 +4,7 @@ Converts coordinates from one build/assembly to another using liftOver binary and mapping files downloaded from UCSC. """

-import sys, os, string +import os, string, subprocess, sys import tempfile import re

@@ -51,15 +51,20 @@ if in_dbkey == "?": stop_err( "Input dataset genome build unspecified, click the pencil icon in the history item to specify it." )

- if not os.path.isfile( mapfilepath ): stop_err( "%s mapping is not currently available." % ( mapfilepath.split('/')[-1].split('.')[0] ) )

safe_infile = safe_bed_file(infile) -cmd_line = "liftOver -minMatch=" + str(minMatch) + " " + safe_infile + " " + mapfilepath + " " + outfile1 + " " + outfile2 + " > /dev/null 2>&1" +cmd_line = "liftOver -minMatch=" + str(minMatch) + " " + safe_infile + " " + mapfilepath + " " + outfile1 + " " + outfile2 + " > /dev/null" try: - os.system( cmd_line ) -except Exception, exc: - stop_err( "Exception caught attempting conversion: %s" % str( exc ) ) + # have to nest try-except in try-finally to handle 2.4 + try: + proc = subprocess.Popen( args=cmd_line, shell=True, stderr=subprocess.PIPE ) + returncode = proc.wait() + stderr = proc.stderr.read() + if returncode != 0: + raise Exception, stderr + except Exception, e: + raise Exception, 'Exception caught attempting conversion: ' + str( e ) finally: os.remove(safe_infile) diff -r f84112d155c0 -r 9efe896dbb17 tools/extract/liftOver_wrapper.xml --- a/tools/extract/liftOver_wrapper.xml Wed Mar 10 11:28:50 2010 -0500 +++ b/tools/extract/liftOver_wrapper.xml Wed Mar 10 11:51:00 2010 -0500 @@ -23,19 +23,16 @@ <requirements> <requirement type="binary">liftOver</requirement> </requirements> - <tests> <test> <param name="input" value="5.bed" dbkey="hg18" ftype="bed" /> - <param name="to_dbkey" value="/galaxy/data/hg18/liftOver/hg18ToPanTro2.over.chain" /> + <param name="to_dbkey" value="panTro2" /> <param name="minMatch" value="0.95" /> <output name="out_file1" file="5_liftover_mapped.bed"/> <output name="out_file2" file="5_liftover_unmapped.bed"/> </test> </tests> - <help> - .. class:: warningmark

Make sure that the genome build of the input dataset is specified (click the pencil icon in the history item to set it if necessary). @@ -71,6 +68,6 @@ chrX 158279 160020 AK097346 0 + chrX 160024 169033 AK074528 0 -

-</help> -<code file="liftOver_wrapper_code.py"/> + </help> + <code file="liftOver_wrapper_code.py" /> </tool> diff -r f84112d155c0 -r 9efe896dbb17 tools/extract/liftOver_wrapper_code.py --- a/tools/extract/liftOver_wrapper_code.py Wed Mar 10 11:28:50 2010 -0500 +++ b/tools/extract/liftOver_wrapper_code.py Wed Mar 10 11:51:00 2010 -0500 @@ -1,8 +1,10 @@ +import os + def exec_before_job(app, inp_data, out_data, param_dict, tool): #Assuming the path of the form liftOverDirectory/hg18ToHg17.over.chain (This is how the mapping chain files from UCSC look.) - to_dbkey = param_dict['to_dbkey'].split('.')[0].split('To')[1] + #allows for . in path + to_dbkey = os.path.split(param_dict['to_dbkey'])[1].split('.')[0].split('To')[1] to_dbkey = to_dbkey[0].lower()+to_dbkey[1:] out_data['out_file1'].set_dbkey(to_dbkey) out_data['out_file1'].name = out_data['out_file1'].name + " [ MAPPED COORDINATES ]" out_data['out_file2'].name = out_data['out_file2'].name + " [ UNMAPPED COORDINATES ]" - diff -r f84112d155c0 -r 9efe896dbb17 tools/samtools/sam_pileup.py --- a/tools/samtools/sam_pileup.py Wed Mar 10 11:28:50 2010 -0500 +++ b/tools/samtools/sam_pileup.py Wed Mar 10 11:51:00 2010 -0500 @@ -78,30 +78,32 @@ #prepare basic pileup command cmd = 'samtools pileup %s -f %s %s > %s' try: - #index reference if necessary and prepare pileup command - if options.ref == 'indexed': - if not os.path.exists( "%s.fai" % seqPath ): - raise Exception, "No sequences are available for '%s', request them by reporting this error." % options.dbkey - cmd = cmd % ( opts, seqPath, tmpf0bam_name, options.output1 ) - elif options.ref == 'history': - os.symlink( options.ownFile, tmpf1_name ) - cmdIndex = 'samtools faidx %s' % ( tmpf1_name ) - proc = subprocess.Popen( args=cmdIndex, shell=True, cwd=tmpDir, stderr=subprocess.PIPE ) + # have to nest try-except in try-finally to handle 2.4 + try: + #index reference if necessary and prepare pileup command + if options.ref == 'indexed': + if not os.path.exists( "%s.fai" % seqPath ): + raise Exception, "No sequences are available for '%s', request them by reporting this error." % options.dbkey + cmd = cmd % ( opts, seqPath, tmpf0bam_name, options.output1 ) + elif options.ref == 'history': + os.symlink( options.ownFile, tmpf1_name ) + cmdIndex = 'samtools faidx %s' % ( tmpf1_name ) + proc = subprocess.Popen( args=cmdIndex, shell=True, cwd=tmpDir, stderr=subprocess.PIPE ) + returncode = proc.wait() + stderr = proc.stderr.read() + #did index succeed? + if returncode != 0: + raise Exception, 'Error creating index file\n' + stderr + cmd = cmd % ( opts, tmpf1_name, tmpf0bam_name, options.output1 ) + #perform pileup command + proc = subprocess.Popen( args=cmd, shell=True, cwd=tmpDir, stderr=subprocess.PIPE ) returncode = proc.wait() + #did it succeed? stderr = proc.stderr.read() - #did index succeed? if returncode != 0: - raise Exception, 'Error creating index file\n' + stderr - cmd = cmd % ( opts, tmpf1_name, tmpf0bam_name, options.output1 ) - #perform pileup command - proc = subprocess.Popen( args=cmd, shell=True, cwd=tmpDir, stderr=subprocess.PIPE ) - returncode = proc.wait() - #did it succeed? - stderr = proc.stderr.read() - if returncode != 0: - raise Exception, stderr - except Exception, e: - stop_err( 'Error running Samtools pileup tool\n' + str( e ) ) + raise Exception, stderr + except Exception, e: + stop_err( 'Error running Samtools pileup tool\n' + str( e ) ) finally: #clean up temp files if os.path.exists( tmpDir ): diff -r f84112d155c0 -r 9efe896dbb17 tools/sr_mapping/bowtie_wrapper.py --- a/tools/sr_mapping/bowtie_wrapper.py Wed Mar 10 11:28:50 2010 -0500 +++ b/tools/sr_mapping/bowtie_wrapper.py Wed Mar 10 11:51:00 2010 -0500 @@ -2,7 +2,7 @@

""" Runs Bowtie on single-end or paired-end data. -For use with Bowtie v. 0.12.1 +For use with Bowtie v. 0.12.3

usage: bowtie_wrapper.py [options] -t, --threads=t: The number of threads to run @@ -58,12 +58,12 @@ -H, --suppressHeader=H: Suppress header """

-import optparse, os, shutil, sys, tempfile +import optparse, os, shutil, subprocess, sys, tempfile

def stop_err( msg ): - sys.stderr.write( "%s\n" % msg ) + sys.stderr.write( '%s\n' % msg ) sys.exit() - + def __main__(): #Parse Command Line parser = optparse.OptionParser() @@ -119,6 +119,7 @@ parser.add_option( '-x', '--indexSettings', dest='index_settings', help='Whether or not indexing options are to be set' ) parser.add_option( '-H', '--suppressHeader', dest='suppressHeader', help='Suppress header' ) (options, args) = parser.parse_args() + stdout = '' # make temp directory for placement of indices and copy reference file there if necessary tmp_index_dir = tempfile.mkdtemp() # get type of data (solid or solexa) @@ -187,17 +188,25 @@ iseed, icutoff, colorspace ) except ValueError: indexing_cmds = '%s' % colorspace + ref_file = tempfile.NamedTemporaryFile( dir=tmp_index_dir ) + ref_file_name = ref_file.name + ref_file.close() + os.symlink( options.ref, ref_file_name ) + cmd1 = 'bowtie-build %s -f %s %s' % ( indexing_cmds, ref_file_name, ref_file_name ) try: - shutil.copy( options.ref, tmp_index_dir ) + proc = subprocess.Popen( args=cmd1, shell=True, cwd=tmp_index_dir, stderr=subprocess.PIPE, stdout=subprocess.PIPE ) + returncode = proc.wait() + stderr = proc.stderr.read() + if returncode != 0: + raise Exception, stderr except Exception, e: - stop_err( 'Error creating temp directory for indexing purposes\n' + str( e ) ) - options.ref = os.path.join( tmp_index_dir, os.path.split( options.ref )[1] ) - cmd1 = 'bowtie-build %s -f %s %s 2> /dev/null' % ( indexing_cmds, options.ref, options.ref ) - try: - os.chdir( tmp_index_dir ) - os.system( cmd1 ) - except Exception, e: + # clean up temp dir + if os.path.exists( tmp_index_dir ): + shutil.rmtree( tmp_index_dir ) stop_err( 'Error indexing reference sequence\n' + str( e ) ) + stdout += 'File indexed. ' + else: + ref_file_name = options.ref # set up aligning and generate aligning command options # automatically set threads in both cases if options.suppressHeader == 'true': @@ -328,19 +337,34 @@ best, strata, offrate, seed, colorspace, snpphred, snpfrac, keepends, options.threads, suppressHeader ) except ValueError, e: + # clean up temp dir + if os.path.exists( tmp_index_dir ): + shutil.rmtree( tmp_index_dir ) stop_err( 'Something is wrong with the alignment parameters and the alignment could not be run\n' + str( e ) ) - # prepare actual aligning commands - if options.paired == 'paired': - cmd2 = 'bowtie %s %s -1 %s -2 %s > %s 2> /dev/null' % ( aligning_cmds, options.ref, options.input1, options.input2, options.output ) - else: - cmd2 = 'bowtie %s %s %s > %s 2> /dev/null' % ( aligning_cmds, options.ref, options.input1, options.output ) - # align try: - os.system( cmd2 ) - except Exception, e: - stop_err( 'Error aligning sequence\n' + str( e ) ) - # clean up temp dir - if os.path.exists( tmp_index_dir ): - shutil.rmtree( tmp_index_dir ) + # have to nest try-except in try-finally to handle 2.4 + try: + # prepare actual aligning commands + if options.paired == 'paired': + cmd2 = 'bowtie %s %s -1 %s -2 %s > %s' % ( aligning_cmds, ref_file_name, options.input1, options.input2, options.output ) + else: + cmd2 = 'bowtie %s %s %s > %s' % ( aligning_cmds, ref_file_name, options.input1, options.output ) + # align + proc = subprocess.Popen( args=cmd2, shell=True, cwd=tmp_index_dir, stderr=subprocess.PIPE ) + returncode = proc.wait() + stderr = proc.stderr.read() + if returncode != 0: + raise Exception, stderr + # check that there are results in the output file + if len( open( options.output, 'rb' ).read().strip() ) == 0: + raise Exception, 'The output file is empty, there may be an error with your input file or settings.' + except Exception, e: + stop_err( 'Error aligning sequence. ' + str( e ) ) + finally: + # clean up temp dir + if os.path.exists( tmp_index_dir ): + shutil.rmtree( tmp_index_dir ) + stdout += 'Sequence file aligned.\n' + sys.stdout.write( stdout )

if __name__=="__main__": __main__() diff -r f84112d155c0 -r 9efe896dbb17 tools/sr_mapping/bwa_wrapper.py --- a/tools/sr_mapping/bwa_wrapper.py Wed Mar 10 11:28:50 2010 -0500 +++ b/tools/sr_mapping/bwa_wrapper.py Wed Mar 10 11:51:00 2010 -0500 @@ -152,55 +152,57 @@ cmd3 = 'bwa samse %s %s %s %s >> %s' % ( gen_alignment_cmds, ref_file_name, tmp_align_out_name, options.fastq, options.output ) # perform alignments try: - # align + # need to nest try-except in try-finally to handle 2.4 try: - proc = subprocess.Popen( args=cmd2, shell=True, cwd=tmp_dir, stderr=subprocess.PIPE ) - returncode = proc.wait() - stderr = proc.stderr.read() - if returncode != 0: - raise Exception, stderr - except Exception, e: - raise Exception, 'Error aligning sequence. ' + str( e ) - # and again if paired data - try: - if cmd2b: - proc = subprocess.Popen( args=cmd2b, shell=True, cwd=tmp_dir, stderr=subprocess.PIPE ) + # align + try: + proc = subprocess.Popen( args=cmd2, shell=True, cwd=tmp_dir, stderr=subprocess.PIPE ) returncode = proc.wait() stderr = proc.stderr.read() if returncode != 0: raise Exception, stderr + except Exception, e: + raise Exception, 'Error aligning sequence. ' + str( e ) + # and again if paired data + try: + if cmd2b: + proc = subprocess.Popen( args=cmd2b, shell=True, cwd=tmp_dir, stderr=subprocess.PIPE ) + returncode = proc.wait() + stderr = proc.stderr.read() + if returncode != 0: + raise Exception, stderr + except Exception, e: + raise Exception, 'Error aligning second sequence. ' + str( e ) + # generate align + try: + proc = subprocess.Popen( args=cmd3, shell=True, cwd=tmp_dir, stderr=subprocess.PIPE ) + returncode = proc.wait() + stderr = proc.stderr.read() + if returncode != 0: + raise Exception, stderr + except Exception, e: + raise Exception, 'Error generating alignments. ' + str( e ) + # remove header if necessary + if options.suppressHeader == 'true': + tmp_out = tempfile.NamedTemporaryFile( dir=tmp_dir) + tmp_out_name = tmp_out.name + tmp_out.close() + try: + shutil.move( options.output, tmp_out_name ) + except Exception, e: + raise Exception, 'Error moving output file before removing headers. ' + str( e ) + fout = file( options.output, 'w' ) + for line in file( tmp_out.name, 'r' ): + if not ( line.startswith( '@HD' ) or line.startswith( '@SQ' ) or line.startswith( '@RG' ) or line.startswith( '@PG' ) or line.startswith( '@CO' ) ): + fout.write( line ) + fout.close() + # check that there are results in the output file + if os.path.getsize( options.output ) > 0: + sys.stdout.write( 'BWA run on %s-end data' % options.genAlignType ) + else: + raise Exception, 'The output file is empty. You may simply have no matches, or there may be an error with your input file or settings.' except Exception, e: - raise Exception, 'Error aligning second sequence. ' + str( e ) - # generate align - try: - proc = subprocess.Popen( args=cmd3, shell=True, cwd=tmp_dir, stderr=subprocess.PIPE ) - returncode = proc.wait() - stderr = proc.stderr.read() - if returncode != 0: - raise Exception, stderr - except Exception, e: - raise Exception, 'Error generating alignments. ' + str( e ) - # remove header if necessary - if options.suppressHeader == 'true': - tmp_out = tempfile.NamedTemporaryFile( dir=tmp_dir) - tmp_out_name = tmp_out.name - tmp_out.close() - try: - shutil.move( options.output, tmp_out_name ) - except Exception, e: - raise Exception, 'Error moving output file before removing headers. ' + str( e ) - fout = file( options.output, 'w' ) - for line in file( tmp_out.name, 'r' ): - if not ( line.startswith( '@HD' ) or line.startswith( '@SQ' ) or line.startswith( '@RG' ) or line.startswith( '@PG' ) or line.startswith( '@CO' ) ): - fout.write( line ) - fout.close() - # check that there are results in the output file - if os.path.getsize( options.output ) > 0: - sys.stdout.write( 'BWA run on %s-end data' % options.genAlignType ) - else: - raise Exception, 'The output file is empty. You may simply have no matches, or there may be an error with your input file or settings.' - except Exception, e: - stop_err( 'The alignment failed.\n' + str( e ) ) + stop_err( 'The alignment failed.\n' + str( e ) ) finally: # clean up temp dir if os.path.exists( tmp_index_dir ):