For my Biology class, I made a Python script which takes a DNA sequence as input, translates it into an mRNA sequence, and then again into a tRNA sequence. It then matches each mRNA codon with an amino acid, and gives the user all of the data it produces.
Since this program may have to work with large amounts of DNA code, I just want some advice to see what could be done to make the program run faster and more efficiently.
Here is the 'symbols.p' file and here is the 'mRNA_to_protein.p' file.
import pickle
'''
Program takes a DNA genetic code sequence in as input.
It then translates the DNA code into mRNA code, and again into tRNA code.
After that, it matches each mRNA codon with an amino acid, as found in the hash table inside the pickle file.
It then matches each amino acid with its symbol, and prints all the data onto the screen.
'''
def main():
# Asks the user if they would like to open a file which contains their genetic sequence.
# Removes all whitespace from input in order to process it correctly.
open_choice = remove_spaces(input("Do you want to load a file to translate [Y/N]").upper())
# Processes whether the user wants to use a file
while open_choice != 'Y' and open_choice != 'N':
open_choice = remove_spaces(input("Do you want to load a file to translate [Y/N]").upper())
if open_choice == 'Y':
sequence = get_file().upper()
else:
sequence = input("Enter the DNA sequence to convert it: ").upper() # Gets the DNA sequence to convert from input, if the user
# declines to open a file.
sequence = remove_spaces(sequence) # Removes spaces from the user's sequence
while not check_sequence(sequence, 'dna'): # Sends to check sequence function to confirm that it is a valid sequence
sequence = input("Please enter a correct sequence: ").upper() # If sequence is invalid, repeat until it is valid
sequence = remove_spaces(sequence)
original_sequence = ' '.join([sequence[i:i + 3] for i in range(0, len(sequence), 3)]) # Saves original DNA sequence
mRNA = convert_sequence(sequence, 'dna') # Saves mRNA sequence
tRNA = convert_sequence(remove_spaces(mRNA), 'rna') # Saves tRNA sequence
proteins = convert_to_proteins((mRNA + " ")) # Prints amino acid sequence
symbols = convert_symbols(proteins) # Prints amino acid symbols
print('DNA: ' + original_sequence) # Prints original sequence
print('mRNA: ' + mRNA) # Prints mRNA sequence
print('tRNA: ' + tRNA) # Prints tRNA sequence
print(" ".join(proteins))
print(" ".join(symbols))
dump_data(original_sequence, mRNA, tRNA, " ".join(proteins), " ".join(symbols))
input()
# Checks sequence for validility
def check_sequence(sequence, type): # Takes the sequence input and the type of sequence
if type == 'rna': # If it is an RNA sequence, confirm it only contains characters in AUCG
a = 'AUCG'
else:
a = 'ATCG' # If it is an DNA sequence, confirm it only contains characters in ATCG
sequence_list = list(sequence) # Converts sequence into a list
# Checks each character in list to see if it is in respective character list determined above
for i in sequence_list:
if i not in a: # If a character is invalid, return False
return False
return True # If all characters are valid, return True
# Converts sequence to rNA
def convert_sequence(sequence, sequence_type): # Takes sequence and type of secuence
if sequence_type == 'dna': # if the sequence is DNA: convert t to u
conversion_dict = {
'A': 'U',
'T': 'A',
'C': 'G',
'G': 'C'
}
else: # if the sequence is RBA: convert u to a
conversion_dict = {
'A': 'U',
'U': 'A',
'C': 'G',
'G': 'C'
}
# convert sequence into a list
converted_sequence = []
sequence_list = list(sequence)
# convert list one by one, checking the dictionary for the corresponding key, and add it to the new clist
for i in sequence_list:
converted_sequence.append(conversion_dict[i])
# return converted sequence, seperated by a space every three spaces
converted_sequence = ''.join(converted_sequence)
# noinspection PyTypeChecker
return ' '.join([converted_sequence[i:i + 3] for i in range(0, len(converted_sequence), 3)])
def convert_to_proteins(sequence):
n = []
protein_sequence = []
mrna_to_protein = pickle.load(open('mRNA_to_protein.p', 'rb'))
for i in sequence:
if not i.isspace():
n.append(i)
else:
if len(n) < 3:
break
protein_sequence.append(mrna_to_protein[''.join(n)])
n = []
return protein_sequence
def convert_symbols(proteins):
symbol_list = []
symbols = pickle.load(open('symbols.p', 'rb'))
for i in proteins:
symbol_list.append(symbols[i])
return symbol_list
# removes all spaces in a sequence
def remove_spaces(x):
return (''.join(x.split())).strip()
def get_file():
file_name = input("Enter file name: ")
while True:
try:
f = open(file_name, 'r')
sequence = f.read()
while not check_sequence(remove_spaces(sequence).upper(), 'dna'):
file_name = input("\nPlease provide a file with a correct DNA sequence: ")
break
except FileNotFoundError:
file_name = input("\nThe file '{}' was not found. \nPlease enter an accurate file name/path: ".format(file_name))
return sequence
def dump_data(dna, mrna, trna, aa, s):
file = open('results.txt', 'w')
file.write('DNA: ' + dna + "\n")
file.write('mRNA: ' + mrna + "\n")
file.write('tRNA: ' + trna + "\n")
file.write(aa + "\n")
file.write(s + "\n")
return True
main()
1 Answer 1
There's a couple of high level problems with your code:
You should remove your comments. They're as useful as the code they're commenting as they're 'micro' comments.
print(mRNA)
is without a doubt printing mRNA. If there was a comment saying that you use lists in both forms,['abc', 'def']
and'abc def'
, then that would have been a good comment.They should be used to describe oddities in your code. Or if we are using optimized code to say what the code is doing, from a high-level perspective.
You should order your code so you can read down the file, and know most things about the code at that point. To fully understand
main
you need to know every other function, and so that should go to the bottom of the file.You should use
if __name__ == '__main__'
.You should pick one quotation mark style. And stick with it.
'
or"
.
The above are simple and easy to fix. But you also have harder to fix problems, which require more effort to fix.
You duplicate the code to 'chunk' strings into lists. This is when you change
'abcdef'
to either'abc def'
or['abc', 'def']
. And so you should make this a function.You should change
check_sequence
to takea
rather thantype
as an argument. This allows greater control on the function. And allows you to perform an abstraction on your code.You can then move these global inputs to global constants, and use them when needed. Global constants mean that you're less likely to make a typo and unknowingly break your program.
You could change
check_sequence
to use a comprehension andall
to reduce the amount of lines. It can also improve clarity as then you don't need to explicitly returnTrue
orFalse
.You should use arrays rather than strings. As said above you use all three formats,
'abcdef'
,'abc def'
, and['abc', 'def']
. Instead you should only use the last format. This allows you to get easier to read code, and removes the need forn
inconvert_to_proteins
.As stated above you should change
convert_sequence
to take a list. This can allow you to use a list comprehension to build the list with minimal characters. You can also change your code to usestr.translate
rather than implementing the translation yourself.You can change both
convert_to_proteins
andconvert_symbols
to be the same function. Movingpickle.load
out of the function, and by passing lists, simplify the code to just a list comprehension, where you're indexing a dictionary.You can move your print to
dump_data
as you're printing and writing the same data. This allows you to useformat
to build the string once, and for you to print it and write it.Always use
with
when you useopen
. It closes the stream when you're done with it, and without can lead to bugs and errors.Your code to get user input is confusing, and duplicates logic. Your option for reading from a file or from user input is good. You code initially reads that a user could pick to read from a file, but then if the file is malformed it reads from user input. On further reading you find out it's not possible, which is a problem for readers.
And so if you implement all the above you can get:
import pickle
TRANSLATION_DNA = {'A': 'U', 'T': 'A', 'C': 'G', 'G': 'C'}
TRANSLATION_OTHER = {'A': 'U', 'U': 'A', 'C': 'G', 'G': 'C'}
VALID_INPUT_RNA = 'AUCG'
VALID_INPUT_OTHER = 'ATCG'
def grouper(sequence, n):
return [sequence[i:i + n] for i in range(0, len(sequence), n)]
def check_sequence(sequence, valid_input):
return all(i in valid_input for i in sequence)
def translate_sequence(sequence, conversion_dict):
table = {ord(k): v for k, v in conversion_dict.items()}
return [g.translate(table) for g in sequence]
def convert_sequence(sequence, conversion_dict):
return [conversion_dict[i] for i in sequence]
def dump_data(*args):
output = 'DNA: {}\nmRNA: {}\ntRNA: {}\n{}\n{}'.format(*map(' '.join, args))
print(output)
with open('results.txt', 'w') as f:
f.write(output + '\n')
def safe_read(file_name):
with open(file_name, 'rb') as f:
return pickle.load(f)
def convert(sequence):
mrna_to_protein = safe_read('mRNA_to_protein.p')
protein_symbols = safe_read('symbols.p')
original_sequence = grouper(sequence, 3)
mRNA = translate_sequence(original_sequence, TRANSLATION_DNA)
tRNA = translate_sequence(mRNA, TRANSLATION_OTHER)
proteins = convert_sequence(mRNA, mrna_to_protein)
symbols = convert_sequence(proteins, protein_symbols)
return original_sequence, mRNA, tRNA, proteins, symbols
def read_sequence():
while True:
file_name = input('Enter file name: ')
try:
with open(file_name, 'r') as f:
return f.read()
except FileNotFoundError:
print('File {!r} not found.'.format(file_name))
def remove_spaces(x):
return (''.join(x.split())).strip()
def get_sequence():
while True:
open_choice = input('Do you want to load a file to translate [Y/N]').strip().upper()
if open_choice in ('Y', 'N'):
break
while True:
sequence = (
read_sequence()
if open_choice == 'Y' else
input('Enter the DNA sequence to convert it: ')
)
sequence = remove_spaces(sequence.upper())
if check_sequence(sequence, VALID_INPUT_OTHER):
return sequence
print('Invalid sequence.')
def main():
sequence = get_sequence()
original_sequence, mRNA, tRNA, proteins, symbols = convert(sequence)
dump_data(original_sequence, mRNA, tRNA, proteins, symbols)
input()
if __name__ == '__main__':
main()
-
\$\begingroup\$ Do you think that doing
dump_data(convert(get_sequence()))
inmain
would be too compact? \$\endgroup\$ChatterOne– ChatterOne2016年11月15日 12:22:47 +00:00Commented Nov 15, 2016 at 12:22 -
\$\begingroup\$ @ChatterOne Yes you definitely could use
dump_data(*convert(get_sequence()))
. If I edit my program I'd add that in, I just didn't think of it. :) \$\endgroup\$2016年11月15日 12:25:11 +00:00Commented Nov 15, 2016 at 12:25 -
\$\begingroup\$ @Peilonrayz, Thanks for the excellent review. I am having trouble understanding how the
translate_sequence
function works. Could you explain it? \$\endgroup\$vkumar– vkumar2016年11月15日 14:15:52 +00:00Commented Nov 15, 2016 at 14:15 -
1\$\begingroup\$ @vkumar Sure, :) The first line changes the dictionary from say
{"A": "B"}
to{65: "B"}
this is asstr.translate
looks up on the characters Unicode ordinal. If this limitation wasn't imposed bystr.translate
then that line wouldn't be there. The second line performs the translation on all the items in the list, as we're inputting it in the form['abc', 'def']
rather than'abcdef'
. And is roughly equivalent to[''.join(conversion_dict[i] for i in g) for g in sequence]
. \$\endgroup\$2016年11月15日 14:57:10 +00:00Commented Nov 15, 2016 at 14:57
Explore related questions
See similar questions with these tags.