Convert #hashtagstylephrases to a proper list of words I'm writing a program to split the words contained in an hashtag.

For example I want to split the hashtags:

#Whatthehello #goback


into:

What the hello go back




import re,pdb
words = []
# corncob_lowercase.txt contains a list of dictionary words
with open('corncob_lowercase.txt', 'rb') as f:
    for read_word in f:
        words.append(read_word.strip())
def func_replace(each_func):
    i = 0
    wordsineach_func = []
    while len(each_func) &gt; 0:
        i = i + 1
        word_found = longest_word(each_func)
        if len(word_found) &gt; 0:
            wordsineach_func.append(word_found)
            each_func = each_func.replace(word_found, "")
    return ' '.join(wordsineach_func)


def longest_word(phrase):
    phrase_length = len(phrase)
    words_found = []
    index = 0
    outerstring = ""
    while index &lt; phrase_length:
        outerstring = outerstring + phrase[index]
        index = index + 1
        if outerstring in words or outerstring.lower() in words:
            words_found.append(outerstring)
    if len(words_found) == 0:
        words_found.append(phrase)
    return max(words_found, key=len)

def check_hashtager(s):
    pdb.set_trace()
    return re.sub(r"#(\w+)", lambda m: func_replace(m.group(1)), s)

if __name__ == '__main__':
    print check_hashtager("#Whatthehello #goback")


I ran this on huge string which >10k hashtags. The function calls make it slow...

Edit 1:
There is something weird that happens when I ran it "#Whatthehello #goback #1". It doesn't stop.
At first, let me say that the code is somewhat neat and reads easily. A few nitpicks though, based upon PEP8:


each import statement should stand on its own line;
you should put blank lines before function declarations;
you should improve your naming, first by putting more _ in your variable names to separate the words, and second by choosing more meaningful names.


Debugging

You should not need to keep debugging directive in your code now. Remove the pdb module.

Global variables

And top-level code

You are aware of the if __name__ == '__main__' practice but you still keep code at top-level. Worse, you rely on your top-level code to populate a global variable… What if I want to import your_file and use your_file.check_hashtager with my own dictionary of french words?

Better let your functions accept the dictionary as parameter and let the user provide the filename:

def partition_hashtag(text, dictionary_filename):
    with open(dictionary_filename) as f:
        words = [word.strip() for word in f]
    return re.sub(r'#(\w+)', lambda m: partitioner(m.group(1), words), text)


A note on variable names: check_xxx has a meaning of validation only, leading the user to expect a boolean return value (or an error raised on validation error). I considered split_hashtag but I feel like partition_hashtag works best, since it is associated with a dictionary to partition from. partitioner is just a logical consequence.

Partitioning words

Let's talk about longest_word a bit for starter:


you check words of increasing length so there is no need to keep them all and retrieve the longest afterwards, just return the last one you found ;
you can iterate over the letters of phrase directly instead of retrieving them by indexes; it's faster.




def longest_word(phrase, words):  # accepting the dictionary of words as parameter
    word_found = None
    outer_string = ''
    for letter in phrase:
        outer_string += letter
        if outer_string in words or outer_string.lower() in words:
            word_found = outer_string
    return word_found


You’ll need to change if len(word_found) &gt; 0 in func_replace by if word_found is not None or even if word_found which you should have used from start.

But there still exist issue with that code. Concatenating strings in a for loop is not the fastest thing in the world, you might want to consider using

for i in range(1, len(phrase)):
    outer_string = phrase[:i]


instead. But more importantly, checking existence in a list is an O(n) operation, n being the length of the list; you will have a faster execution if words were a set instead because the same operation runs in O(1).

One last thing to note is that you are checking shorter words before longer ones. If you were to try your phrase by decreasing length, the first matching word would be the one you are looking for:

def longest_word(phrase, words):
    current_try = phrase
    while current_try:
        if current_try in words or current_try.lower() in words:
            return current_try
        current_try = current_try[:-1]

    # if nothing was found, return the original phrase
    return phrase


Now let's tackle how this code is called.

First of, what is this i exactly? It is incremented but never used. Second, you made sure in longest_word to return words of at least 1 letter so you don't need to test the length. Third, you don't need to modify your input string in place but only drop its head corresponding to the word you just found.

def partitioner(hashtag, words):
    extracted_words = []
    while hashtag:
        word_found = longest_word(hashtag, words)
        extracted_words.append(word_found)
        hashtag = hashtag[len(word_found):]
    return ' '.join(extracted_words)


Introducing generators

What if partitioner produced only the list of words? We could call the join operation out of this function, like

lambda m: ' '.join(partitioner(m.group(1), words))


This let us focus on only producing words efficiently inside partioner.

Even though the proposed functions reads clearer and should execute faster, there is that list-building-out-of-a-loop thing that is discouraged in Python. Instead we try to come up with list comprehensions to build it faster (plus it reads better). Meaning we usually turn

a_list = []
for element in an_other_list:
    a_list.append(some_function(element))


into

a_list = [some_function(e) for e in an_other_list]


Here the while loop has side-effects on hashtag so we can not directly use that. But…

We are not actually interested in a list of words. We are only interested in getting words one by one to feed them into join. And Python lets you do exactly that by turning your function into a generator: use the yield keyword instead of return. Each time the control flow reach a yield keyword, a result will be returned to the caller. And if the caller ask for more, the control flow will resume after the exact same yield, remembering its old state.

Proposed improvements

import re


def partitioner(hashtag, words):
    while hashtag:
        word_found = longest_word(hashtag, words)
        yield word_found
        hashtag = hashtag[len(word_found):]

def longest_word(phrase, words):
    current_try = phrase
    while current_try:
        if current_try in words or current_try.lower() in words:
            return current_try
        current_try = current_try[:-1]
    # if nothing was found, return the original phrase
    return phrase

def partition_hashtag(text, words):
    return re.sub(r'#(\w+)', lambda m: ' '.join(partitioner(m.group(1), words)), text)

def read_dictionary_file(filename):
    with open(filename, 'rb') as f:
        return set(word.strip() for word in f)


if __name__ == '__main__':
    words = read_dictionary_file('corncob_lowercase.txt')
    print partition_hashtag("#Whatthehello #goback", words)