[MRG] DOC Avoid missing reference warnings, instead displaying as <code>#12970
[MRG] DOC Avoid missing reference warnings, instead displaying as <code>#12970thomasjpfan wants to merge 10 commits intoscikit-learn:masterfrom
Conversation
doc/conf.py
Outdated
| def filter_any_references(record): | ||
| msg = record.getMessage() | ||
| return ("'any' reference target not found:" not in msg and | ||
| "more than one target found for 'any' cross-reference" not in msg) |
There was a problem hiding this comment.
Can you give examines of this second error from the logs please?
There was a problem hiding this comment.
scikit-learn/sklearn/compose/_column_transformer.py:docstring of
sklearn.compose.make_column_transformer:15:
WARNING: more than one target found for 'any' cross-reference 'fit':
could be :std:term: or :py:meth: or :py:meth: ...
Looking at the html, this fit reference does not actually reference anything. I suspect we want this to reference to the glossary.
This PR was updated to remove the second error filter, given it is a valid error.
jnothman
left a comment
There was a problem hiding this comment.
Tbh I've not yet checked the logs, but...
|
I am actually not fully sure this is the best solution. You could also say that all those warnings are indicating an error in the docs and could be fixed? In any case, this will also hide actual errors like a typo in something that you expected to link to the glossary or API docs. |
|
It may well hide true errors, but the number of false positives is much
greater than the number of false negatives. And the `any` default role is
explicit designed to back off to code-style font in case of no target.
|
That's not fully the case. At least, it has a different "look" than normal code with double backticks. It is in bold (and to me looks like missing link, but that's probably because I know too much about sphinx :-)). |
|
The best solution would be to go through the warnings and make sure we use single backticks to actually reference something. Double backticks would be used to highlight code (with the gray background) that does not reference something. |
|
I have began the process of reviewing the docs and only use single backticks if they are referencing something in #12992. |
|
My preference would be to not have to say "double backticks" and explain
what I mean in a review ever again. I think making single backticks work
okay, even with some errors, is the lowest-maintenance option.
|
This goal is reasonable. For reference, the warning: WARNING: more than one target found for 'any' cross-reference 'fit':
could be :std:term: or :py:meth:appears because |
|
Will it try to point it to the local method, or to the first/last-defined
fit anywhere??
|
|
It will always point to the glossary, because sphinx will choose the first reference it finds, which is For reference, here is the sphinx code that does this: https://github.com/sphinx-doc/sphinx/blob/7ffd6ccee8b0c6316159c4295e2f44f8c57b90d6/sphinx/transforms/post_transforms/__init__.py#L136-L141 |
|
#13000 should help resolve the issue with |
|
The latest update changes the 'any' roles that does not have any references to appear like a normal inline code element. Consider this page generated by this PR, the |
|
In addition to removing the warnings, this PR essentially makes single backticks act like double backticks when a reference is not found. |
|
|
||
| def maps_any_missing_reference_to_code_block(app, env, node, contnode): | ||
| if node['reftype'] == 'any': | ||
| contnode['classes'] = [] |
There was a problem hiding this comment.
Can you explain (and perhaps add a comment) how this results in code-like styling? Thanks.
jnothman
left a comment
There was a problem hiding this comment.
I've re-titled the PR. I hope that's alright.
|
I am closing this PR in favor of the solution in #13000. |
Reference Issues/PRs
Fixes #12802, See also #12535
What does this implement/fix? Explain your changes.
Maps references that are not found to a code block.