improvement for speed and clarity #1
Conversation
* phrases as tuples instead of lists * can use set with tuple of strings to get unique phrases
2 similar comments
2 similar comments
| @@ -166,20 +166,12 @@ def _generate_phrases(self, sentences): | |||
| @return: List of List of strings where each sublist is a collection | |||
There was a problem hiding this comment.
Would you please change the @return to
@return: List of string tuples where each tuple is a collection of words forming a contender phrase.
so that code change and corresponding documentation are part of a single commit.
There was a problem hiding this comment.
Here we should maybe even say @return: Set of string tuples... because the return value is now a set instead of a list.
| unique_phrase_list.append(phrase) | ||
| return unique_phrase_list | ||
| phrase_list.update(self._get_phrase_list_from_words(word_list)) | ||
| return phrase_list | ||
|
|
||
| def _get_phrase_list_from_words(self, word_list): | ||
| '''Method to create contender phrases from the list of words that form a |
There was a problem hiding this comment.
Please change
List of phrases: [['red', 'apples'], ['good'], ['flavour']]
in the function definition to
List of phrases: [('red', 'apples'), ('good'), ('flavour')]
to reflect the change.
| @@ -53,13 +53,15 @@ def test_generate_phrases(self): | |||
| ['define'], ['sequence'], ['one'], ['words'], | |||
| ['provide'], ['compact', 'representation'], ['document'], | |||
| ['content']] | |||
| phrase_list = set(tuple(phrase) for phrase in phrase_list) | |||
There was a problem hiding this comment.
As the expected set is a hard coded list of phrases, maybe its a good idea to create the set in line 52 as a set of tuples rather than taking another pass at it here. Same goes for the other two changes. If you choose not to do it, I will do it myself.
There was a problem hiding this comment.
I'll do it, I was just lazy about it when updating tests.
Hi, and thanks for writing the RAKE algorithm using NLTK.
I've noticed in the code that you keep phrases as lists of words, which then makes it more difficult to compute the list of unique phrases. What I changed was to use tuples instead of lists, then the list of phrases can be a set instead and you get unique phrases with less code, and faster as well.