TL;DR: See end of answer for updated code with suggested improvements
While I don't have many suggestions regarding nested for-loops, as you only seem to have two of them, I do have a few other suggestions:
Suggestions
1. Use with
for file opening
You use try/finally to open/close your files, and while you're absolutely right to close your files, Python's with
statement takes care of this for you. Another lesser-known characteristic of the with
statement is that you can "nest" them in a single line, which is useful in your case.
So, I would recommend updating
try:
input_file = open(input_file_path)
output_file = open(output_file_path, 'w')
... # Do stuff
finally:
input_file.close()
output_file.close()
to
with open(input_file_path) as input_file, open(output_file_path, "w") as output_file:
... # Do stuff
When the context of the with
statement is exited, your opened files will automatically be closed.
2. Clean up variable naming
Python convention is to use lower snake-case for normal variable names and function names - Camel-case (upper) is usually only used for class definitions. Therefore, I would recommend renaming your checkSequenceCharacters
and checkSequenceLength
functions to check_sequence_characters
and check_sequence_length
, respectively. This might be considered nitpicking, but maintaining consistent naming conventions always makes things easier for me.
3. Use str in
str, instead of checking individual equality
In check_sequence_characters
, you can more easily ensure that each character in seq
is in an allowed set of characters using the following:
if any(c not in "AGTC" for c in seq):
raise ValueError("Not a valid sequence (A/G/T/C ONLY)")
4. Raise Exceptions, instead of printing "ERROR"
There are a few places in your example where you print error messages. That's fine if you really don't want to raise actual exceptions, but that does seem to be the behavior your code is dancing around. I would recommend reading Python's documentation on raise
if you're unfamiliar with it.
5. Use if __name__ == "__main__"
when running module as source
If you don't know what if __name__ == "__main__"
means, check out this explanation. Basically, my recommendation is to start using this pattern even if it may not be strictly necessary in your example (although it does protect you from the lurking seq
name-shadowing bug). This will facilitate maintenance of your code should you need to import your example code as a module in the future, for example.
6. Use enumerate
to loop through input_file
with line index
Instead of using the line_location
variable, you can remove the line_location = 0
line, and change
for line in input_file:
line_location = line_location + 1
to
for line_location, line in enumerate(input_file):
If you still want line_location
to be 1-indexed, just add 1 when you print
/write
it below.
Other Notes
- For more recent versions of Python, you'll need to use
input
instead of raw_input
- If there is any chance your output file has not already been created by you (which seems likely), you should
open
it using mode="x"
, which will create and open the file
- If any of the subdirectories in the output file path have not already been created, you'll need to use something like
os.makedirs
to handle directory creation
- The line
print("[*] FILE CREATED AT: " + output_file_path)
near the bottom may be misleading, as the output file is created when you call open
, not when you call write
. This means that even if count == 0
, an output file was created (albeit a blank one)
- In the provided suggested code, I removed the
lines
variable, since it isn't used; however, if you actually do need it for something not shown in your example, it can be safely re-added, along with the lines.append(line)
line
- I would recommend embracing Python's string
format
method, instead of using +
to construct strings
TL;DR
If you decide to follow the above suggestions, your code will probably end up looking something like this:
import os
import re
def check_sequence_characters(seq):
if any(c not in "AGTC" for c in seq):
raise ValueError("Not a valid sequence (A/G/T/C ONLY)")
def check_sequence_length(seq):
if len(seq) != 5:
raise ValueError("Has to be exactly 5 letters long!")
def _execute():
seq = raw_input("Enter a 5 basepair sequence using only A,G,T,C: \n")
count = 0
check_sequence_characters(seq)
check_sequence_length(seq)
input_file_path = os.getenv("HOME") + "/Desktop/sequencer/inputfile.txt"
output_file_path = os.getenv("HOME") + "/Desktop/sequencer/output/" + seq + ".txt"
with open(input_file_path) as input_file, open(output_file_path, 'x+') as output_file:
for line_location, line in enumerate(input_file):
arr = re.findall(seq, line)
if arr and arr[0] == seq:
count += 1
print("[+] Sequence found at line " + str(line_location + 1))
output_file.write("Line " + str(line_location + 1) + ": " + line + "\n")
if count != 0:
print("[*] Matches: " + str(count))
print("[*] FILE CREATED AT: " + output_file_path)
else:
raise RuntimeError("No matches found!")
if __name__ == "__main__":
_execute()