Categorizing gene sequences read from a CSV file

Question

I am relatively new to programming and would love to get some feedback on the following section of my code.

class Gene:
    def __init__(self, gene_symbol, gene_id):
        # gene_symbol represents the abbreviated name of the gene (string)
        # Example: 'RHO' (short for 'Rhodopsin')
        self.gene_symbol = gene_symbol

        # gene_id represents an accession ID for the gene (string)
        # Example: 'NM_000539.3'
        self.gene_id = gene_id

        # is_valid_ functions check if the type of the gene_id is valid
        # and returns True or False
        # Assume that the types of the gene_id are mutually exclusive
        if is_valid_refseq(gene_id):
            # Example of a valid refseq: 'NM_000539.3'
            self.gene_id_type = REFSEQ

        elif is_valid_ensembl_gene(gene_id):
            # Example of a valid ensembl_gene: 'ENSG00000163914'
            self.gene_id_type = ENSEMBL_GENE

        elif is_valid_ensembl_transcript(gene_id):
            # Example of a valid ensembl_transcript: 'ENST00000296271'
            self.gene_id_type = ENSEMBL_TRANSCRIPT

        else:
            raise InvalidGeneIDError("Invalid gene_id: {}".format(gene_id))



refseqs = []
ensembl_genes = []
ensembl_transcripts = []

with open(csv_gene_list, 'r', newline='') as csv_input:

    reader = csv.reader(csv_input, delimiter=',')
    next(reader)
    for row in reader:
        row_gene_symbol = row[0]
        row_gene_id = row[1]

        row_gene = Gene(row_gene_symbol, row_gene_id)

        if row_gene.gene_id_type == REFSEQ:
            refseqs.append(row_gene)

        elif row_gene.gene_id_type == ENSEMBL_GENE:
            ensembl_genes.append(row_gene)

        elif row_gene.gene_id_type == ENSEMBL_TRANSCRIPT:
            ensembl_transcripts.append(row_gene)

        else:
            # What do I do here?
            raise AssertionError('Unrecognized gene_id_type: {}'.format(
                    row_gene.gene_id_type))

Assume gene_symbol and gene_id are strings and the is_valid_ functions are implemented elsewhere in my code.

I would like to ask if implementing these multiple if-else statements is the right approach. I feel like my code might get cluttered if I add more of these conditional expressions down the line.

Another question is on the very last else statement. What do I do here in a case like this? If the code works as intended, then it shouldn't reach the last else statement because an InvalidGeneIDError should've been raised when initializing the Gene object. One solution is to omit this part and change elif row_gene.gene_id_type == ENSEMBL_TRANSCRIPT: to else:, but I would much rather explicitly state the specific check to help convey the intentions of my code.

My main priority as I move forward is to focus on developing good coding style/habits and to write readable code. I would also like to learn/discover new tools/tricks that Python has to offer. Any suggestions and help would be greatly appreciated.

Answer 1

Yeah, the problem is the switch: you have the same set of if-elif-else conditions duplicated in two places. Chances are that as your program grows, you will add more replicas of these chains. The problem with these chains is that if later you need to add one more case in the middle, you will need to modify all replicas accordingly. This is error prone, as you might not remember all the places where you replicated the chains.

How can we deal with that better? Can we replace these chains with something else, in a way that if we add a new case in the Gene constructor, we can be free from worrying about the rest of the program?

At least in this example, there is a workable solution. Consider these 3 lists:

refseqs = []
ensembl_genes = []
ensembl_transcripts = []

These lists are an important part of the problem. They mirror the currently supported 3 types, and when you put items in these lists, you need to refer to these lists by these names exactly.

Instead of these lists, you could use a dictionary of lists, where the keys are the gene types, and the values are lists. Something like this:

genes = dict()

with open(csv_gene_list, 'r', newline='') as csv_input:

    reader = csv.reader(csv_input, delimiter=',')
    next(reader)
    for row in reader:
        row_gene_symbol = row[0]
        row_gene_id = row[1]

        row_gene = Gene(row_gene_symbol, row_gene_id)

        if row_gene.row_gene_id not in genes:
            genes[row_gene.row_gene_id] = []

        genes[row_gene.row_gene_id].append(row_gene)
        # ...

In this form the chain is gone, and the second part of your question is naturally gone too.

As @jaime pointed out in a comment, this can be greatly simplified:

if row_gene.row_gene_id not in genes:
    genes[row_gene.row_gene_id] = []

genes[row_gene.row_gene_id].append(row_gene)

Using setdefault like this:

genes.setdefault(row_gene.row_gene_id, []).append(row_gene)

Answer 2

Here is a revision to my code after taking in the suggestion made by @janos.

# Make an empty dictionary that will hold the following key-value pairs
# {gene_id_type: list of Gene objects}
all_genes = dict()

with open(csv_gene_list, 'r', newline='') as csv_input:

    reader = csv.reader(csv_input, delimiter=',')
    next(reader)
    for row in reader:
        row_gene_symbol = row[0]
        row_gene_id = row[1]

        row_gene = Gene(row_gene_symbol, row_gene_id)
        gene_id_type = row_gene.gene_id_type

        # Check if gene_id_type is in the dictionary
        if gene_id_type in all_genes:
            # Append row_gene to the list in the dictionary
            all_genes[gene_id_type].append(row_gene)

        else:
            # Make a new key named gene_id_type and
            # assign its value as a list with row_gene in it
            all_genes[gene_id_type] = [row_gene]

# Do stuff with all_genes

Answer 3

If the rows in your file should only contain two elements then you could set both symbol and id at once. I also don't think you need to have the row_ prefix.

gene_symbol, gene_id = row

This will unpack each row into the two values for gene_symbol and gene_id. It's a cleaner way of getting both values at once. It will raise an error if row has more than two values, but that also means you don't silently ignore malformed data.