-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC Fix warnings about references and links #14976
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
Changes from all commits
a6b9305
a8a9511
0b54ff1
26bbdec
586a36f
cc0208e
a33af98
549cea6
19b997c
6a6015e
1084747
bdd700f
765e0fe
a1ddbc3
52352bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -274,8 +274,8 @@ class LatentDirichletAllocation(TransformerMixin, BaseEstimator): | |
|
||
References | ||
---------- | ||
[1] "Online Learning for Latent Dirichlet Allocation", Matthew D. Hoffman, | ||
David M. Blei, Francis Bach, 2010 | ||
.. [1] "Online Learning for Latent Dirichlet Allocation", Matthew D. | ||
Hoffman, David M. Blei, Francis Bach, 2010 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we make such bibliographic references with number indices, we then have indentifier conflicts that automatically get resolved by sphinx using a hash of the text of the reference: Which leads to: in the body of the docstring when the reference is cited. So we have two options:
For short class / function docstrings both options are possible. Whenever the text is long to the point the reader has to scroll by more than 1 screen length, I believe the second option makes most sense. Updating all references to follow the second option (explicit identifiers) would lead to a large PR, so if we want to do this I would do it in several small localized PRs that can be merged progressively, starting with the PRs that actually cause sphinx warnings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer the second option. But then all refs (even those not linked in the text) must be updated for consistency (and future use). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ogrisel, do you mind if I address the citing problem in a new issue? Just to focus here on the sphinx warnings... and close that one ASAP? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that the core of the referencing problem is also reported there #4344 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree let's focus on fixing the sphinx warnings first and see about the general consistency of references across the full code-base for later PRs. |
||
|
||
[2] "Stochastic Variational Inference", Matthew D. Hoffman, David M. Blei, | ||
Chong Wang, John Paisley, 2013 | ||
|
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.
Maybe add something like. "This value is typically required to compute asymmetric evaluation metrics such as precision and recall".
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.
Added in bdd700f