-
-
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
LSI documentation #1892
LSI documentation #1892
Conversation
… the LsiModel module. Types need to be checked before merging
…cted shape for sparse matrix arguments
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.
Overall - looks great! Please make several fixes & annotate lsi_distributed.py
file too.
gensim/models/lsimodel.py
Outdated
@@ -91,30 +99,81 @@ def clip_spectrum(s, k, discard=0.001): | |||
|
|||
|
|||
def asfarray(a, name=''): | |||
""" | |||
Return an array laid out in Fortran order in memory. |
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.
"""Some word
instead of
"""
Some word
...
gensim/models/lsimodel.py
Outdated
|
||
Returns | ||
------- | ||
out : ndarray |
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.
numpy.ndarray
+ we have no "out" parameter here (only a
), this is enough to mention the only type (here and everywhere).
""" | ||
Construct the (U, S) projection from a corpus `docs`. The projection can | ||
be later updated by merging it with another Projection via `self.merge()`. | ||
"""Construct the (U, S) projection from a 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.
It's better to move this description to class docstring (instead of __init__
), here and everywhere.
gensim/models/lsimodel.py
Outdated
|
||
Returns | ||
------- | ||
Projection |
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.
please use links in sphinx manner, like
:class:`~gensim.models.lsimodel.Projection`
for all "gensim types" (i.e. defined in gensim), here and everywhere.
* General class level remarks moved to class docstring from `__init__` * References to `gensim` classes are now using `sphinx` notation. * Numpy parameters annotated with `np.type` instead of `type`.
gensim/models/lsi_dispatcher.py
Outdated
@@ -5,12 +5,18 @@ | |||
# Licensed under the GNU LGPL v2.1 - http://www.gnu.org/licenses/lgpl.html | |||
|
|||
""" | |||
USAGE: %(program)s SIZE_OF_JOBS_QUEUE | |||
Dispatcher process which orchestrates distributed LSI computations. Run this |
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.
This is executable script, some "special case", please look into __doc__
and .. program-output
from 117d447 (same for lsi_worker)
Goond job @steremma 🔥 |
* Added numpy style docstrings to all functions, methods and classes in the LsiModel module. Types need to be checked before merging * Fixed generic container type (stream/list -> iterable) and added expected shape for sparse matrix arguments * Fix PEP-8 * Applied corrections mentioned in code review. * General class level remarks moved to class docstring from `__init__` * References to `gensim` classes are now using `sphinx` notation. * Numpy parameters annotated with `np.type` instead of `type`. * Added docstrings for `lsi_worker` and `lsi_dispatcher` * added argument parsing and fixed __doc__ * update configs with new extension sphinxcontrib.programoutput * added blank link in __doc__ * sphinx identation fix * chmod revert * fix lsimodel[1] * fix lsimodel[2] * fix lsimodel[3] * fix lsimodel[4] * fix basemodel * fixes * fix lsi_worker & missing fields in .rst * last fixes for worker & dispatcher * add missing link
In this PR I attempt to fix the source internal documentation for the LSI model. More specifically:
Numpy docstrings for functions methods and classes
Including type annotation for arguments and returned objects