Skip to content
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 docstrings for gensim.models.logentropy_model #1803

Merged
merged 7 commits into from
Jan 11, 2018

Conversation

minggli
Copy link
Contributor

@minggli minggli commented Dec 19, 2017

Motivation RE: https://gist.github.com/menshikh-iv/075824fdc4906a3b1b74b8f94ce64b59

This PR improves and corrects the documentations of https://github.com/RaRe-Technologies/gensim/blob/develop/gensim/models/logentropy_model.py

Changes include:

  • clarify and separate the behaviours of construct and initialize. Existing docs were not as accurate.
  • describe parameters of public methods.
  • miscellaneous such as docstrings at module level.
  • minor styling for imports separating standard library imports.

Copy link
Contributor

@menshikh-iv menshikh-iv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@minggli: +1, please add small fixes according to my comments.

@@ -3,17 +3,18 @@
#
# Licensed under the GNU LGPL v2.1 - http://www.gnu.org/licenses/lgpl.html

"""This module contains LogEntropyModel class"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add more description "what is it"

  • small description with links
  • executable example

@@ -33,10 +34,11 @@ class LogEntropyModel(interfaces.TransformationABC):

The main methods are:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example is better than words :)


Parameters
----------
corpus : iterable object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iterable of list of (int, int), here and everywhere

corpus : iterable object
itertable consisting of word-documents that are made up of term id and term frequency.
normalize : bool, optional
`normalize` dictates whether the resulting vectors will be set to unit length.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalize -> parameter, also it will be good if you present both cases

if True, ..., otherwise ....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@minggli
Copy link
Contributor Author

minggli commented Dec 20, 2017

Changes implemented. @menshikh-iv

@menshikh-iv menshikh-iv changed the title improve and correct documentation of models/logentropy_model Fix docstrings for gensim.models.logentropy_model Dec 20, 2017
@menshikh-iv
Copy link
Contributor

Great @minggli, I'll look into on next week, please wait for a little.

@menshikh-iv
Copy link
Contributor

Need to resolve #1807 first (because we have formulas in this PR that should be rendered with latex)

@menshikh-iv
Copy link
Contributor

Thank you @minggli, congratz with the first contribution to gensim 🥇

@menshikh-iv menshikh-iv merged commit a1fd854 into piskvorky:develop Jan 11, 2018
@minggli
Copy link
Contributor Author

minggli commented Jan 11, 2018

pleasure. finally giving something back to the community.

@minggli minggli deleted the docs/logentopy_model branch January 14, 2018 01:08
sharanry added a commit to sharanry/gensim that referenced this pull request Jan 15, 2018
* Add wordnet mammal train file for Poincare notebook (piskvorky#1781)

* Adds wordnet mammal train file

* Adds link to data file in notebook

* Fix tox.ini/setup.cfg configuration (piskvorky#1815)

* update according to new pytest_benchmark version

* update wheel-storage url

* use only twine

* Fix docstrings for `gensim.utils` (piskvorky#1797)

* Add docstrings in numpy-style fromat

* fix PEP8

* remove outdated "hack" (smart_open is core dependency right now)

* fix docstrings[1]

* remove unused internal class

* fix docstrings[2]

* fix docstrings[3]

* fix docstrings[4]

* fix docstrings[5]

* fix docstrings[6]

* fix docstrings[7]

* fix docstrings[8]

* add missing `pattern` to doc dependencies

* fix docstrings[9]

* fix docstrings[10]

* Fix docstrings for `gensim.models.rpmodel` (piskvorky#1802)

* first attempt to convert few lines into numpy-style doc

* added parameters in documentation

* more documentation

* few corrections

* show inheritance and undoc members

* show special members

* example is executable now

* link to the paper added, named parameters

* fixed doc

* fixed doc

* fixed whitespaces

* fix docstrings & PEP8

* fix docstrings

* fix typo

* Fix docstrings for `gensim.models.translation_matrix` (piskvorky#1806)

* convert Space class doc to numpy style

* fix docstrings[1]

* fix docstrings[2]

* remove useless load

* fix docstrings[3]

* add missing import

* fix docstrings[4]

* Add CircleCI for build documentation. Fix piskvorky#1807 (piskvorky#1822)

* init config for circle

* change

* rm cache, install tox distinctly

* fix indentation & command

* update venv

* add pip-cache

* add apt packages for latex

* rename

* enable latex rendering

* remove doc building from Travis

* store new doc version

* Refactor API reference `gensim.topic_coherence`. Fix piskvorky#1669 (piskvorky#1714)

* Refactored aggregation

* Micro-Fix for aggregation.py, partially refactored direct_confirmation.py

* Partially refactored indirect_confirmation_measure

* Some additions

* Math attempts

* add math extension for sphinx

* Minor refactoring

* Some refactoring for probability_estimation

* Beta-strings

* Different additions

* Minor changes

* text_analysis left

* Added example for ContextVectorComputer class

* probability_estimation 0.9

* beta_version

* Added some examples for text_analysis

* text_analysis: corrected example for class UsesDictionary

* Final additions for text_analysis.py

* fix cross-reference problem

* fix pep8

* fix aggregation

* fix direct_confirmation_measure

* fix types in direct_confirmation_measure

* partial fix indirect_confirmation_measure

* HotFix for probability_estimation and segmentation

* Refactoring for probability_estimation

* Changes for indirect_confirmation_measure

* Fixed segmentation, partly fixed text_analysis

* Add Notes for text_analysis

* fix di/ind

* fix doc examples in probability_estimation

* fix probability_estimation

* fix segmentation

* fix docstring in probability_estimation

* partial fix test_analysis

* add latex stuff for docs build

* doc fix[1]

* doc fix[2]

* remove apt install from travis (now doc build in circle)

* Fix docstrings for `gensim.models.normmodel` (piskvorky#1805)

* First edits

* changed bow

* Added examples

* Final commit of the night

* Still struggling with docs

* Removed examples but still struggling with documentation

* fix docstring

* fix docstring[2]

* Fix docstrings for `gensim.models.logentropy_model` (piskvorky#1803)

* improve and correct documentation of models/logentropy_model

* include fixes according to comments

* implement fixes suggested

* associate methods with examples.

* fix minor typos

* doc fix

* Fix docstrings for `gensim.matutils` (piskvorky#1804)

* numpy style documentation on matutils.py

* doc fix[1]

* doc fix[2]

* doc fix[3]

* doc fix[4]

* doc fix[5]

* doc fix[6]

* Fix formula in `gensim.summarization.bm25`. Fix piskvorky#1828 (piskvorky#1833)

* bm25 scoring function updated

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828 , Tests added

* Fixes piskvorky#1828 , Tests added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828

* Refactor tests for `gensim.corpora.WikiCorpus`(piskvorky#1821)

* minor style refactoring and comment fixes in accordance to PEP8

* Created test data in legitimate compressed XML format (.xml.bz2) for the WikiCorpus class.

* Used the same raw data found for other sources (9 articles).

* Added Various wiki markup to test the parsing regural expressions

* Added test class for the WikiCorpus source.

* Following the same inheritance schema as in the source TestWikiCorpus > TestTextCorpus > CorpusTestCase.

* Testing methods are overriden where necessary to reflect logic changes.

* All existing functionality is tested (account for markup handling, minimum article length etc)

* Fix python 3 compatibility for generator next method

* code review corrections

* Moved WikiCorpus tests from test/test_wikicorpus.py into its class within the test_corpora.py file.

* Adapted all old tests to the new class

* Current Test class schema ensures that WikiCorpus also passes tests defined in parents

* Deleted test_wikicorpus.py since it is now redundant

* Discarded the empty input test for the WikiCorpus since an empty file is not legitimate XML

* Added 2 more tests

* Fix parameter setting for `FastText.train`. Fix piskvorky#1818 (piskvorky#1837)

* bm25 scoring function updated

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828

* Fixes piskvorky#1828 , Tests added

* Fixes piskvorky#1828 , Tests added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828 , Tests Added

* Fixes piskvorky#1828

* Function Parameters corrected , Fixes piskvorky#1818

* add missing params + add supercall

* Fix positional params used for `gensim.models.CoherenceModel` in `gensim.models.callbacks` (piskvorky#1823)

* add keyword params for call to gensim.models.CoherenceModel as positional arguments for coherence and topn were incorrect due to skipping param for keyed_vectors

* Fix PEP8
sharanry added a commit to sharanry/gensim that referenced this pull request Jan 15, 2018
…into smart_open

* 'develop' of https://github.com/RaRe-Technologies/gensim:
  Fix positional params used for `gensim.models.CoherenceModel` in `gensim.models.callbacks` (piskvorky#1823)
  Fix parameter setting for `FastText.train`. Fix piskvorky#1818 (piskvorky#1837)
  Refactor tests for `gensim.corpora.WikiCorpus`(piskvorky#1821)
  Fix formula in `gensim.summarization.bm25`. Fix piskvorky#1828 (piskvorky#1833)
  Fix docstrings for `gensim.matutils` (piskvorky#1804)
  Fix docstrings for `gensim.models.logentropy_model` (piskvorky#1803)
  Fix docstrings for `gensim.models.normmodel` (piskvorky#1805)
  Refactor API reference `gensim.topic_coherence`. Fix piskvorky#1669 (piskvorky#1714)
  Add CircleCI for build documentation. Fix piskvorky#1807 (piskvorky#1822)
  Fix docstrings for `gensim.models.translation_matrix` (piskvorky#1806)
  Fix docstrings for `gensim.models.rpmodel` (piskvorky#1802)
  Fix docstrings for `gensim.utils` (piskvorky#1797)
  Fix tox.ini/setup.cfg configuration (piskvorky#1815)
  Add wordnet mammal train file for Poincare notebook (piskvorky#1781)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants