-
-
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
Add new API-reference page, automated generation of .rst
files, enable sphinx-gallery
. Fix #1808
#1809
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.
Really great change, big thanks @anotherbugmaste 💣
Of course, need to work more with structure now (missed some things), but you are on the right way!
@@ -32,6 +32,7 @@ help: | |||
|
|||
clean: | |||
-rm -rf $(BUILDDIR)/* | |||
-rm -rf generated/* |
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.
How related $(BUILDDIR)
and generated/
?
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 no way!
generated
is the folder which contains new auto-generated .rst
files. It ought to be purged every time documentation builds, otherwise autosummary
will not update reference page.
@@ -0,0 +1,15 @@ | |||
:mod:`{{module}}`.{{objname}} |
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 add a link in answer, how to "form" this templates?
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.
Done
gensim/summarization/summarizer.py
Outdated
"""Get a list of the most important documents of a corpus using a variation of the TextRank algorithm [1]_. | ||
Used as helper for summarize :func:`~gensim.summarization.summarizer.summarizer` | ||
"""Get a list of the most important documents of a corpus using a | ||
variation of the TextRank algorithm [1]. |
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.
probably, you can use text identifiers for it, for example [some-paper]
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.
AFAIK this bug is somehow related to the trailing underscore in [reference_name]_
. I'll try to find a better workaround.
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.
@anotherbugmaster add links to comment for this PR about this bug, I'll try to find workaround too
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.
Done!
We'll continue work on it together with @anotherbugmaster in Jan |
As for the bug message:
Here's what I've found: https://groups.google.com/forum/#!topic/sphinx-users/JVNPh7AIKi8 |
@anotherbugmaster Just a generic gentle suggestion - Can you please change the title to something more informative? 😄 |
docs/src/Makefile
Outdated
|
||
html: | ||
$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html | ||
rm -r $(BUILDDIR)/html/_sources | ||
cp -r $(BUILDDIR)/html/* ../ | ||
cp -r $(BUILDDIR)/html/* . |
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?
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.
My mistake, I don't like the copying to the upper level though. Why are we doing it in the first place?
@@ -1,3 +1,3 @@ | |||
/*! jQuery Migrate v1.1.1 | (c) 2005, 2013 jQuery Foundation, Inc. and other contributors | jquery.org/license */ | |||
/*! jQuery Migrate v1.1.1 | (c) 2005, 2013 jQuery Foundation, Inc. and other contributors | jquery.org/license */ |
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.
can you simply checkout this file, please?
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.
:D
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.
Sure
gensim/summarization/summarizer.py
Outdated
@@ -344,6 +345,12 @@ def summarize_corpus(corpus, ratio=0.2): | |||
list of str | |||
Most important documents of given `corpus` sorted by the document score, highest first. | |||
|
|||
References | |||
---------- | |||
.. [1] Federico Barrios, Federico L´opez, Luis Argerich, Rosita Wachenchauzer (2016). |
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.
[]
.rst
files, enable sphinx-gallery
. Fix #1808
@@ -301,10 +301,12 @@ def __init__(self, corpus=None, id2word=None, dictionary=None, wlocal=utils.iden | |||
Only when `pivot` is not None will pivoted document length normalization be applied. | |||
Otherwise, regular TfIdf is used. | |||
slope : float, optional | |||
Parameter required by pivoted document length normalization which determines the slope to which | |||
the `old normalization` can be tilted. This parameter only works when pivot is defined. | |||
It is the parameter required by pivoted document length normalization which determines the slope to which |
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 conflict is incorrectly resolved. Please continue with changes in #2102, which should be up-to-date with develop
(plus with some additional fixes).
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.
@piskvorky author (@anotherbugmaster) can't commit to your branch (lack of permissions)
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 not add @anotherbugmaster as a committer? I cannot commit to his fork either (that's why I created another branch, directly in gensim
, so we can all collaborate).
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.
@piskvorky done
@anotherbugmaster please fix #2102 branch (you have permissions for clone current repository (not your fork), commit & push directly to #2102 branch), sorry for bothering on vacation.
Continue in #2102 |
Fix #1808
Reformatted API reference, but there's a little bit ugly workaround there: reference (like
[1]_
) doesn't work correctly if there's another reference with the same number in the upper section. I explored sklearn repo, it seems like they're aware of it and trying not to do so or making the same workaround as me.Autosummary sphinx module
Templating