Mutations and Combinations

Question

I have recently answered one question on Stack Overflow.

Problem statement:

Input sequence:

PEACCEL

User mutation input file:

E2R C4W E6G

The algorithm is for an input sequence of, say, PEACCEL and mutation E2R. We need to replace second character in sequence, that is, E to R, and so on for the combinations of mutations.

I provided this answer:

from itertools import combinations,chain
from collections import Counter
input_sequence='PEACCEL'
mutations=map(lambda x:x.strip(),open('a.txt','r').readlines())
 #['E2R', 'C4W', 'E6G']
mutation_combinations= chain.from_iterable([list(combinations(mutations,i))for i in range(1,4)])
 #[('E2R',), ('C4W',), ('E6G',), ('E2R', 'C4W'), ('E2R', 'E6G'), ('C4W', 'E6G'), ('E2R', 'C4W', 'E6G')]

for i in mutation_combinations:
    print "combination :"+'_'.join(i)
    c=Counter({})
    temp_string=input_sequence
    for j in i:
        c[j[1]]=j[2].lower()
    for index,letter in c.items():
        temp_string=temp_string[:int(index)-1]+letter+temp_string[int(index):]
    print temp_string

Output:

combination :E2R
PrACCEL
combination :C4W
PEAwCEL
combination :E6G
PEACCgL
combination :E2R_C4W
PrAwCEL
combination :E2R_E6G
PrACCgL
combination :C4W_E6G
PEAwCgL
combination :E2R_C4W_E6G
PrAwCgL

Algorithm I followed:

1. Read the mutation sequences like E2R.... from a file using

   mutations=map(lambda x:x.strip(),open('a.txt','r').readlines())
   

2. Made the combinations of the mutations

   mutation_combinations= chain.from_iterable([list(combinations(mutations,i))for i in range(1,4)])
   

   If you have 4 mutations you want all four change the range value to 5.

3. For each combination I replaced them with specified character:

   for j in i:
       c[j[1]]=j[2].lower()
   

   I used the above counter to keep track of which character was to be replaced during the mutation combination.

How can I make this more efficient? I also need to know of any valuable pointers I should know while coding.

Answer 1

There are several things that I would change in your code:

Use functions

This is a good example of spaghetti code where everything is meshed together. You should provide functions to load the mutation list, one to execute one mutation and one that executes a list of mutations.

Printing the results should also be an extra function.

Use a better mutation representation

Your internal representation for the replacements is too leaky (and maybe too easy).

What if the requirements change and you want to replace multiple character strings?

Instead you should use a structure that already does the separation of the search term, the counter and replace term.

Another problem here is that you will not be able to pass replacement indices bigger than 9 because you only take one character instead of matching a regex for the number in the middle and taking the whole number as index.

Missing error handling

Talking of indices, your program will fail when the given index is out of bounds for the input string.

Unclear input

Neither you nor the OP on SO state what the first element in the mutation string is for. I don't see it being used. It seems to duplicate some of the information of the replacement index.

Code formatting

Please improve the code formatting. Take for example this:

temp_string=temp_string[:int(index)-1]+letter+temp_string[int(index):]

I had a hard time finding the boundaries of the [] because everything is so close together. Instead try to leave space around operators/operands:

temp_string = temp_string[:int(index) - 1] + letter + temp_string[int(index):]

Misused collection

I don't see why you are using a Counter in your code. You don't seem to count anything with it and use it like an ordinary dict.

The line

for j in i:
        c[j[1]]=j[2].lower()

Looks dubious, what is it for? If there are several mutation strings with the same offset only the last one will be recorded (because it overwrites the previous ones). Is this the intended behavior?

If you do not want this behavior then why use the dict (or rather Counter) at all? You could have iterated through the mutations and directly applied them with the same result.