[MRG] Warns when reference can not be found.#13365
Merged
adrinjalali merged 5 commits intoscikit-learn:masterfrom Mar 3, 2019
Merged
[MRG] Warns when reference can not be found.#13365adrinjalali merged 5 commits intoscikit-learn:masterfrom
adrinjalali merged 5 commits intoscikit-learn:masterfrom
Conversation
Member
thomasjpfan
commented
Mar 1, 2019
- Warns when reference can not be found with single backticks.
- All glossary links will look like normal links (not bold).
agramfort
approved these changes
Mar 1, 2019
Member
|
Cool ! For the record, we now have 992 new warnings when building the doc. Details |
TomDLT
approved these changes
Mar 2, 2019
| font-size: 1em; | ||
| font-weight: inherit; | ||
| padding: 0; | ||
| } |
Member
There was a problem hiding this comment.
Was there the idea to do this from the python side instead of with css?
(in any case, otherwise some more comments here to explain it would be welcome)
Member
Author
There was a problem hiding this comment.
This PR was updated with a python solution. It forces any roles that resolve to term to have the same html structure as a normal term role.
Member
@TomDLT yes, we had a try to fix all those instances, but that can be a (or more) separate PRs. It touches tons of lines, but it's worth fixing them. |
Member
|
I'm not sure that it's worth warning when reference not found... part of
the point of the single backticks is to allow it to be used for code
elements as a default. I don't think we benefit from this rigorous a doc
linter, either.
|
Member
|
We had a discussion about this on Friday, but should probably have recorded that better on github first. The idea was that (long term) we would use double backticks when we actually know it is code and not a link to the glossary or to a class method. |
Member
Author
|
Another concern I heard was that removing the warnings would make it hard to catch misspelled references. I am 60/40 in favor of having no warnings and would be okay with some misspelled references that turn into code. |
Member
|
Let's warn but set them to ignore by default. That makes it easy to clean
them when we have someone enthusiastic to do so :)
|
Member
Author
This can be done with an environment variable, @adrinjalali @jorisvandenbossche What do you think? |
Member
Author
|
I am now 80/20 for ignoring the warnings by default, with a |
Member
|
Yes, "please use double backticks" is the reason we put in a default_role
at least as much as to get automatic links.
|
xhluca
pushed a commit
to xhluca/scikit-learn
that referenced
this pull request
Apr 28, 2019
* DOC Adds term to column_transformer * DOC Adjusts sphinx resolution * DEV Adds comment * ENH Uses python instead of css
xhluca
pushed a commit
to xhluca/scikit-learn
that referenced
this pull request
Apr 28, 2019
This reverts commit e6c3b7e.
xhluca
pushed a commit
to xhluca/scikit-learn
that referenced
this pull request
Apr 28, 2019
This reverts commit e6c3b7e.
koenvandevelde
pushed a commit
to koenvandevelde/scikit-learn
that referenced
this pull request
Jul 12, 2019
* DOC Adds term to column_transformer * DOC Adjusts sphinx resolution * DEV Adds comment * ENH Uses python instead of css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.