Rewrite porter.py

[ExplodingCabbage]

The commit message sums up most of what I've done here.

Long ago when I was starting out at programming I contributed some pull requests relating to NLTK's version of the Porter stemmer, but did a bad job of it. This is a more thorough attack that eliminates the remaining problems. In particular, it fulfils @stevenbird's request for a more faithful stemmer. (Although this is complicated! See the documentation about modes in the proposed patch.)

Some particular points of concern for reviewers to look at:

• I couldn't get the full test suite to run and, not really knowing how unittest works, had to hack about a bit at the interpreter to run mine. You might want to make sure that they work when run as part of the whole suite and that there's nothing I should've done to add my new tests to the suite that I've missed
• I don't know how docstrings get converted into docs pages at http://www.nltk.org/index.html, so I've just tried to make them as plaintext-readable as possible. They might need tidying up to make them web-ready.
• I've chosen, to preserve backwards-compatibility with the current version of NLTK as much as possible, to use PorterStemmer.NLTK_EXTENSIONS as the default mode. If you care about getting people to use a version of the stemmer consistent with Martin's reference implementations more than you care about backwards compatibility, you might want to use PorterStemmer.MARTIN_EXTENSIONS here instead. I leave it up to you.

[alvations]

+1 Kudos! It looks much better now with the 3 different implementations/extensions.

[ExplodingCabbage]

Quick query since this is now being looked at: was I right to commit the >90000 words of test cases straight into the repo, or should I have put that stuff somewhere else where it could be fetched by the downloader in order to minimise repo size? I didn't look into doing things the latter way; if I should've done, and somebody would like to point be towards docs on how to do so, I'll be happy to comply and rebase.

[alvations]

I think it's best to create a separate CorpusReader object and push it to nltk_data, then load the dataset in the test.

But i'm not the best person to know how this should work. I'm also trying to push some character sets and word lists to test tokenizers and lemmatizers. But I don't think putting them in the core code or test code is appropriate. I would like to know what works best too.

[stevenbird]

@alvations is right, and if it's just a word list we can use the existing corpus reader for that, and I can deal with it myself

[ExplodingCabbage]

I can deal with it myself

To clarify, do you mean that I should do nothing and you will add this stuff to the data repository and rebase my PR yourself to remove the data, or would you like me to take some action?

[alvations]

@ExplodingCabbage I've added new classes to read my word/character lists like this https://github.com/nltk/nltk/pull/1282/files but I'm still not sure whether I'm doing it correctly.

I think @stevenbird will change the corpora readers to fit the nltk.corpus module when you merge the list after creating something like https://github.com/alvations/nltk/blob/develop/nltk/corpus/__init__.py#L280 and https://github.com/alvations/nltk/blob/develop/nltk/corpus/reader/__init__.py#L145 and https://github.com/alvations/nltk/blob/develop/nltk/corpus/reader/wordlist.py#L41

[stevenbird]

@ExplodingCabbage – sorry for the delay. I guess I need to migrate the test files into the NLTK corpus collection before we go any further. I hope to get onto that very soon.

[ExplodingCabbage]

No worries, @stevenbird, life happens and so does prioritisation.

[stevenbird]

@ExplodingCabbage: would you please point me at the test data so I can add it to the NLTK data collection?

[ExplodingCabbage]

@stevenbird:

• https://github.com/ExplodingCabbage/nltk/blob/6ad0304dc80f6653c02bbb5f79cbfbc79a44c0f8/nltk/test/unit/files/porter_vocabulary.txt
• https://github.com/ExplodingCabbage/nltk/blob/6ad0304dc80f6653c02bbb5f79cbfbc79a44c0f8/nltk/test/unit/files/porter_original_output.txt
• https://github.com/ExplodingCabbage/nltk/blob/6ad0304dc80f6653c02bbb5f79cbfbc79a44c0f8/nltk/test/unit/files/porter_nltk_output.txt
• https://github.com/ExplodingCabbage/nltk/blob/6ad0304dc80f6653c02bbb5f79cbfbc79a44c0f8/nltk/test/unit/files/porter_martin_output.txt

[stevenbird]

@ExplodingCabbage: sorry for the delay – the files are now in NLTK Data. Get them with sudo python -m nltk.downloader porter_test. And access them using nltk.data.find, e.g.:

from nltk import data
f = data.find('stemmers/porter_test/porter_nltk_output.txt').open()
f.read().split()
['a', 'aaron', 'abaissiez', 'abandon', 'abandon', 'abas', 'abash', 'abat', 'abat', 'abat', ...]

[ExplodingCabbage]

Roger. Will get this PR ready for merge tonight.

[ExplodingCabbage]

I've rebased 6ad0304 on top of the current develop, creating ca3b475. Will now modify the commit again to remove the test data and use nltk.data.find. Once that's done, this should be ready to merge.

[ExplodingCabbage]

Alright, I've rewritten the commit to obliterate the testing data. When I hackily run the tests by pasting the test class into my Python (2 or 3) shell and running the test_foo methods individually, they pass. I still wasn't able to run the full test suite, unfortunately.

I think this is ready for review and merge?

[stevenbird]

Thanks @ExplodingCabbage, this is great. I'll merge and see how the testing goes on our CI server.