A Very Poetic Program

Question

This program is basically a rhyming dictionary. It makes use of Sphinx's groups phonemic dictionary of English which includes a lot of words broken into their respective phonemes, and if we assume that, at least for most cases, anytime the two last phonemes match, the two words rhyme, then it's very easy to whip up a program that does so. Here's my try at this:

import requests
import os
import re

def load_library():
    with open('library.txt', 'r') as library:
        lines = library.readlines()

    return [x for x in lines if not x.startswith(';;;')]

def rhyme(string):

    if os.path.isfile('library.txt'):
        library = load_library()
    else:
        url = "http://svn.code.sf.net/p/cmusphinx/code/trunk/cmudict/cmudict-0.7b"
        response = requests.get(url, stream=True)

        if not response.ok:
            print("Error loading library")

        with open('library.txt', 'wb+') as library:
            for block in response.iter_content(32):
                library.write(block)

    library = load_library()
    string = string.upper()

    phonemes = [y for y in library if y.startswith(string)][0][len(string):]
    phonemes_split = str.strip(phonemes).split(' ')

    matches = [z for z in library if z.endswith(phonemes_split[-2] + ' ' + phonemes_split[-1] + '\n')]
    matches = [z.strip().split(' ') for z in matches]

    return matches

Of course it's naive to assume that EVERYTIME the last two phonemes of two different words match, they rhyme, but most of the times, they do.

This program returns a list of matching words broken into their respective phonemes.

I spent two years studying English literature before I dropped out in favor of pursuing computer programming so I'm qualified to say that I barely know what a phoneme is.

Answer 1

Library loaded twice

If library.txt exists, it will get loaded twice:

if os.path.isfile('library.txt'):
    library = load_library()
else:
    # download the file ...

library = load_library()

I guess this was just an oversight, with a simple fix:

if not os.path.isfile('library.txt'):
    # download the file ...

library = load_library()

Filter while loading

Instead of loading the library first and then filtering out the commented lines, it would be better to filter as you read:

with open('library.txt', 'r') as library:
    return [line for line in library if not line.startswith(';;;')]

Use generator expressions for better performance

It's a pity to loop over the entire list and filtering when you will only use the first match:

phonemes = [y for y in library if y.startswith(string)][0][len(string):]

You would do better with a generator expression, which is more or less as simple as replacing the surrounding [...] with (...). However, generators are not subscriptable, so to get the first element you must use the next(...) builtin:

phonemes = next(y for y in library if y.startswith(string))[len(string):]

Answer 2

Looks good, I've only got a couple of remarks:

• I'd probably inline the lines in load_library since that still leaves you under 80 characters for the whole line.
• The name of the library appears quite often, extracting that into a e.g. optional parameter or a constant would make sense. Similarly load_library should get the filename as a parameter.
• The url variable is a constant, therefore should be extracted and renamed. I named it URL below, but you should think of a more descriptive name perhaps.
• If the else branch is taken load_library is called twice, that's a bit wasteful. I'd also move the "(download and) load file" into a separate function. The error handling is lacking if the download failed, instead an exception should be raised (perhaps use response.raise_for_status). N.b. the name I chose is terrible, look for a better one.
• The chunk size for the download is also a wee bit small (something like 1-4k bytes is more reasonable), but it probably doesn't matter that much in case the downloaded file is small.
• strip can also be directly called on the string itself. You can also leave out the argument to split if you can split at all whitespace as well. Actually this whole point is already done for matches, so just adapt it to phonemes too).

---

In general it'd probably be better to do more preprocessing of the dictionary file and keep it in memory instead of reparsing it over and over, though of course doing some profiling would be in order for that.

Passing in a preprocessed object (the read library) also makes for a clearer separation between fetching from a URL, disk I/O and finally the actual lookup, which in turn makes testing much easier (because each aspect can be tested independently) and also allows for better reuse (e.g. a function to download from a URL to a file makes sense outside of this particular task).

As a convenience function it shouldn't be a problem if a particular function (for interactive use for example) does all this combined, but in the context of a larger program there are the above concerns in addition to the worse efficiency of redoing these tasks every time the function is called.

---

All in all:

import os
import re
import requests


URL = "http://svn.code.sf.net/p/cmusphinx/code/trunk/cmudict/cmudict-0.7b"


def load_library(filename):
    with open(filename) as library:
        return [x for x in library.readlines() if not x.startswith(';;;')]


def load_or_download_library(filename):
    if not os.path.isfile(filename):
        response = requests.get(URL, stream=True)

        if not response.ok:
            raise Exception("Error loading library")

        with open(filename, 'wb+') as library:
            for block in response.iter_content(32):
                library.write(block)

    return load_library(filename)


def rhyme(string, filename='library.txt'):
    library = load_or_download_library(filename)
    string = string.upper()

    phonemes = [y for y in library if y.startswith(string)][0][len(string):]
    phonemes_split = phonemes.strip().split(' ')

    matches = [z for z in library if z.endswith(phonemes_split[-2] + ' ' + phonemes_split[-1] + '\n')]

    return [z.strip().split(' ') for z in matches]