Remove __getitem__ code duplication in gensim.models.phrases
#2206
Conversation
There was a problem hiding this comment.
Good start, thanks @jenishah. Please fix mentioned issues & make sure that all CI passed successfully (now all ok except flake8 check in Travis).
|
Thanks for the review @menshikh-iv . This was my first PR, and I need to learn a lot about proper documentation. Your reviews were a great help. |
There was a problem hiding this comment.
@jenishah only small things missing until I can merge this, please finalize PR, good work 👍
gensim/models/phrases.py
Outdated
| @@ -232,6 +232,51 @@ def load(cls, *args, **kwargs): | |||
| return model | |||
|
|
|||
|
|
|||
| def _sentence2token(phrase_class, sentence): | |||
| """ returns tokens after from a sentence by joining the phrases with delimiter. | |||
There was a problem hiding this comment.
Should be in form "Do X", not "does X"
gensim/models/phrases.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| {list of str, :class:`gensim.interfaces.TransformedCorpus`} |
There was a problem hiding this comment.
missed ~ for TransformedCorpus (as in other examples)
| {list of str, :class:`gensim.interfaces.TransformedCorpus`} | ||
| `sentence` with detected phrase bigrams merged together, or a streamed corpus of such sentences | ||
| if the input was a corpus. | ||
| """ |
There was a problem hiding this comment.
Empty line at the end of docstring
|
Nice work, thanks @jenishah, congratz with first contribution and happy hacktoberfest 🌟 |
menshikh-iv
changed the title
__getitem__ code duplication in gensim.models.phrases
menshikh-iv
changed the title
__getitem__ code duplication in gensim.models.phrases__getitem__ code duplication in gensim.models.phrases
Fix #2149
wrote a single function
sentence2tokenthat can be used by_getitem_of Phrase and Phrases class.