-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix documentation for various modules #2096
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catches! But please reverse the bool params. The "If True" is both unnecessary and harder to read.
gensim/corpora/dictionary.py
Outdated
@@ -463,7 +458,7 @@ def save_as_text(self, fname, sort_by_word=True): | |||
fname : str | |||
Path to output file. | |||
sort_by_word : bool, optional | |||
Sort words in lexicographical order before writing them out? | |||
Sort words in lexicographical order before writing them out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1: bool should be a question, to make it clear what positive answer means (True).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
gensim/corpora/wikicorpus.py
Outdated
dictionary : :class:`~gensim.corpora.dictionary.Dictionary`, optional | ||
Dictionary, if not provided, this scans the corpus once, to determine its vocabulary | ||
(this needs **really long time**). | ||
(**IMPORTANT: this needs really long time**). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's important, it shouldn't be in brackets :)
really long time
=> a really long time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
gensim/corpora/wikicorpus.py
Outdated
@@ -546,7 +528,11 @@ def __init__(self, fname, processes=None, lemmatize=utils.has_pattern(), diction | |||
token_max_len : int, optional | |||
Maximal token length. | |||
lower : bool, optional | |||
Convert all text to lower case? | |||
Convert all text to lower case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1: should be a question (here and elsewhere).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
gensim/utils.py
Outdated
|
||
Yields | ||
------ | ||
list OR np.ndarray | ||
`chunksize`-ed chunks of elements from `corpus`. | ||
"chunksize"-ed chunks of elements from `corpus`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? chunksize
is a parameter name (just like corpus
, not "corpus"
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in Sphinx, construction
`something`other
without space between "`" and "o" break render & build
- fix bug in smartirs_normalize (old version correct!) - remove persistence test & remove old models from repo (by rename reason)
f57966d
to
a569b76
Compare
Reviewing the remaining high-profile modules (utils, corpora, interfaces).