nltk/nltk — pull request #364 by ExplodingCabbage (2013-03-02 21:00:08+00:00)
Title: Same improvements to the Porter stemmer as before, but without the bugs

A while back I sent a pull request with a bunch of improvements to the porter stemmer. Unfortunately, I also introduced two bugs, one of which was found by stevenbird, who reverted the merge. See here:
https://github.com/nltk/nltk/pull/303

I found time today to fix the issue stevenbird found, which was easy (it was a silly typo). I wasn't yet comfortable, however; I thought I'd tested thoroughly before (I compared the output of the old and revised stemmers for every word in the King James Bible, as well as a few hundred thousand randomly generated strings of alphabet characters, and found that their outputs matched in all cases) but as it turned out I'd still managed to miss something and push a bug. So I looked for a source of all English words to test on, and realized that one exists on all Unix-based operating systems at `/usr/share/dict/words`. I threw together the following test script...

from nltk.stem import porter, oldporter
stem1 = porter.PorterStemmer()
stem2 = oldporter.PorterStemmer()

for word in open('/usr/share/dict/words').read().split('\n'):
try:
res1 = stem1.stem(word)
res2 = stem2.stem(word)
except:
print word
raise
assert res1 == res2, word

... and promptly found another new bug I'd introduced, which was that the stemmer was throwing an exception on the 20 'words' that consist of a vowel, followed by either of the suffixes 'ed' and 'ing', optionally followed by an 's'. For example, it threw an exception on 'oed' and 'iings'. (To be fair to myself, the old version of the stemmer was only avoiding disaster by fortuitous coincidence in those same circumstances - you can see the detail by working through the old code in your head or on paper with 'oed' as the input, but to summarize, it was trying to get the character second-from-last character of a length n string by getting character [n-2], which in the case of a length 1 string was evaluating to character [-1], which due to how Python handles negative indexes was getting the final character, which by simple luck ultimately yielded the right result in the particular 20 cases in which the bug could arise. When I tweaked this in my original refactor to ask for character [-2] instead of character [n-2], the result was an IndexError in these 20 cases instead of flukily doing something reasonable.)

Once I fixed that issue, though, I was able to run through all of /usr/share/dict/words without any exceptions and with the output of the revised stemmer matching the previous version at every stage.

I'm sorry and fairly embarrassed about having pushed something broken before; I guess my idea of what was 'thorough enough' when it came to testing was wrong. I've gone into detail about my testing this time around for precisely that reason. I'm pretty certain now, though, that my latest offering is bug-free, and remains worth accepting for all the same reasons mentioned in the original buggy pull request at https://github.com/nltk/nltk/pull/303 (the most important of which, IMO, is that it makes the stemmer stateless and thus threadsafe, protecting people from unexpected seemingly random errors if they try to share a single stemmer between multiple threads).

Regards,
Cabbage