Extract three letter pattern from a given list

Question

The code takes a list of names and extracts the three letter pattern from the names. Then return the list of names as a value in the dictionary with key as a pattern. The list name has a fixed naming pattern like three letter Spp in the example. It can extent like App, Bpp, Cpp etc..

How can I improve this code?

import sys
import re

def uniquelist(scaffold_names_list):
    unique_names = []
    for name in scaffold_names_list:
        pattern_name = re.search(r"[A-Z]{1}[a-z]{2}", name)
        unique_names.append(pattern_name.group())
    unique_names = list(set(unique_names))
    return unique_names

def uniquepattern(scaffold_names_list):
    unique_names = uniquelist(scaffold_names_list)
    dct = {}

    for singlepattern in unique_names:
        dct['%s' % singlepattern] = []

    for singelelist in scaffold_names_list:
            number = re.search(r"[A-Z]{1}[a-z]{2}", singelelist)
            number = number.group()

            for singlepattern in unique_names:
                if number == singlepattern:
                    dct[singlepattern].append(singelelist)

    return dct


scaffold_names_list = ['Spp1Aa1', 'Spp1Aa1', 'Spp2Aa1', 'Spp3Aa1', 'Spp4Aa1', 'Spp5Aa1']
patterns = uniquepattern(scaffold_names_list)
print(patterns)

{'Spp': ['Spp1Aa1', 'Spp1Aa1', 'Spp2Aa1', 'Spp3Aa1', 'Spp4Aa1', 'Spp5Aa1']}

Answer 1

Naming

I have two problems with your naming:

First, you don't use underscores to separate words. Instead of uniquepattern, use unique_pattern.

Second, your names don't actually tell me anything! Functions, particularly non-method functions, should include a verb unless the verb is implicitly get or is. And implicit is needs to be used with some caution.

So rather than unique_pattern, the name should be some phrase reflective of what your function does. Since you mention "scaffold names", perhaps that function should be group_scaffold_names_by_unique_pattern?

Or maybe the unique pattern has another name? Like prefix? (See my question in the comments.) So perhaps it should be group_scaffold_names_by_prefix?

Is that long? Yes. Does it says what happens? Also, yes. Clarity for the win!

Redundancy. Repetition. Duplication. Multiple Copies.

In coding, if you find yourself typing something for the second time, STOP! Whatever this is needs to be stored once and referenced from different places. Write a function. Store text in a variable. Create a class. Do whatever it takes, but DON'T REPEAT YOURSELF. (This is called the "DRY" principle.)

This code is repeated:

    pattern_name = re.search(r"[A-Z]{1}[a-z]{2}", name)

What does it do? Well, it appears to be used to extract the first matching pattern as the "key" for each name in the list.

So let's write a function for this:

def extract_key(name):
    """ Extract unique pattern from a scaffold name. 
        Apparently, patterns are just the first three characters,
        so don't use regexes, just grab the substring.
    """        
    return name[:3]

Use the standard library

You have a separate function that pulls out the keys (unique patterns) from the names, and returns a uniquified list of them, which you apparently use only to initialize the keys of a dictionary.

Instead of doing that, just use collections.defaultdict to create a dictionary that starts with empty lists:

import collections

groups = collections.defaultdict(list)

With that code, you can be sure that groups[key] is a list (unless you explicitly override it with some non-list value). So let's write your function again:

import collections
from typing import Dictionary, Iterable, List

def group_scaffold_names_by_prefix(names: Iterable[str]) -> Dictionary[str, List[str]]:
    """ Given a list of scaffold names, return a dictionary that
        groups them together by their prefix.
    """
    groups = collections.defaultdict(list)

    for name in names:
        groups[extract_key(name)].append(name)

    return groups

Answer 2

Naming: According to PEP8, you should make all these names snake_case. This is good, but the important bits here are:

1. Do the same thing consistently
2. Have a clear word seperator.

Basically, this means you can use snake_case, camelCase, or even TitleCase, but the last one is generally used only for classes. I'm personally partial to camelCase for functions/methods and snake_case for variables, but snake_case for both is more common, and that's what I'll be using here.

Most important, naming should be meaningful. What does this function do? It parses a list of strings, and returns part of those strings. You've called these strings names, so we'll use something like that.

Also, keep names short. This makes it easier to read.

You first build a list, append to it, convert to a set, then convert to a list again to make them unique. In this case, I'd recommend to start out with a set. The role of this function is to figure out unique prefixes though - not full names.

def get_prefixes(long_names):
    short_names = set()  # set instead of list.
    for name in long_names:
        short_names.add(name[:3])  # Just slice a string if prefix length is constant.
    return list(unique_names)  # Use sorted() instead of list if you care about that.
    # Can return right away. No need to reassign it first.

What else is easily noticeable? This is a rather straightforward loop. Python has a special tool to make this operation a bit easier: list/set/dict/generator comprehensions. Making this a set comprehension later cast to a list:

def get_prefixes(long_names):
    return list({name[:3] for name in long_names})
# If you insist on regex:
def get_prefixes(long_names):
    return list({re.search(r"[A-Z]{1}[a-z]{2}", name) for name in long_names})

That's a lot more shorter, and others reading your code can read it easier.

@Austin Hastings here goes into stdlib usage. If you're looking for performance, that's what you should do. If you're looking to write good production code, that's what you should do. However, I'm going to assume part of this is learning how to code, so I'll rewrite this function.

def uniquepattern(names):
    [other stuff happens]
    for singlepattern in unique_names:
        dct['%s' % singlepattern] = []

Why do string % formatting here? There's no reason, since that list already contains strings and only strings. You can just use it as key. Or even better, combine it with the dict declaration:

    dct = {key, [] for key in unique_names}

Now lets have a look at filling that dict:

    for singelelist in names:  # Spelling error here. 
            number = re.search(r"[A-Z]{1}[a-z]{2}", singelelist)
            number = number.group()

            for singlepattern in unique_names:
                if number == singlepattern:
                    dct[singlepattern].append(singelelist)

That's a double iteration. I really can't figure out why we need that, since we can easily grab the prefixes from the names themselves:

    for name in names:
        dct[name[:3]].append(name)

And even if we need to use regexes to accomodate for variable prefix length, it's easy:

    for name in names:
        dct[re.search(r"[A-Z]{1}[a-z]{2}", name).group()].append(name)

This single loop means that we don't use unique_names more than once, which means we can merge it into where it's used.

So for the entire function we get a much more modest:

def name_sorter(names):
    dct = {key: [] for key in get_prefixes(names)}
    for name in names:
        dct[name[:3]].append(name)
    return dct

And they say python code can't be concise...

So our full script would become:

def get_prefixes(long_names):
    return list({name[:3] for name in long_names})

def name_sorter(names):
    dct = {key: [] for key in get_prefixes(names)}
    for name in names:
        dct[name[:3]].append(name)
    return dct

if __name__ == "__main__":
    scaffold_names_list = ['Spp1Aa1', 'Spp1Aa1', 'Spp2Aa1', 'Spp3Aa1', 'Spp4Aa1', 'Spp5Aa1']
    patterns = name_sorter(scaffold_names_list)
    print(patterns)

Note that I included a if __name__ == "__main__": guard. This means that other scripts can import this file without actually running any code, only making the functions available. But if any script is executed directly, python sets the __name__ variable to the string "__main__", and it is executed. This is widely considered good practice.