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

docs: autorefs warning about rst reference #1495

Closed
pawamoy opened this issue Dec 13, 2024 · 3 comments
Closed

docs: autorefs warning about rst reference #1495

pawamoy opened this issue Dec 13, 2024 · 3 comments

Comments

@pawamoy
Copy link
Contributor

pawamoy commented Dec 13, 2024

From #1494 (comment):

  • when rendering API docs, we enable automatic summaries (summary: true in mkdocs.yml)
  • previously this was an Insiders feature, and it was recently made available to everyone, so it's now active in this project too
  • when we render the extensions sub-package, we summarize each submodule
  • to summarize items, we render the first line of their docstring
  • in the case of the admonition module, its first line contains a reference: [rST][]
  • since we only render this first line, the reference is unresolved by Python-Markdown itself, and autorefs kicks in (and fails too)

Possible solutions:

  • don't use "defered inline references" (not sure what to call them) in docstrings summaries (the first line) and instead put their URL directly within them (so [rST](https://...) instead of [rST][]), or move them lower, into the docstring body
  • use pymdownx.snippets to auto-append references at the end of each markdown document (done on the fly, no modification to any file, except maybe to remove all refs from everywhere and gather them in the single auto-appended file, see https://facelessuser.github.io/pymdown-extensions/extensions/snippets/#auto-append-snippets)
  • disable auto-summaries

Answering to #1494 (comment):

As an aside, if you are rendering a partial document, then you need to account for reference links and properly render them. IMO there should be no requirement that the document author needs to account for this and alter the way links are formatted.

If we want to account for references, then we'll have to parse (actually convert) the whole document (docstring) with Python-Markdown, then only keep the HTML part we're interested in, which in this case is the first line. Not sure how much sense "first line" makes in HTML. It would probably translate as "first paragraph". Which if we want to do in a robust manner, would mean either parsing HTML again, or using a temporary Python-Markdown extension (probably a tree processor) that prunes everything after the first paragraph. This could be wasteful too (converting the whole document just to get rid of most of it). Not sure if there are ways to make this more efficient.

  1. Reference links are resolved.

Implies the above.

  1. Links are striped from summaries and only the label text is included.

Doesn't seem possible. The non-natively-resolved reference could a valid autorefs cross-reference. In this case we do want the warning.

I'd be tempted to say that this is a limitation of the summary feature, and that users should account for it rather than the opposite. The same thing happens for rendering docstring sections by the way, since docstrings written following the Google-style or Numpydoc-style will be split into sections (and sections into items), and each section (item description) will be rendered separately, meaning each section (item) must include its own references and footnotes for them to resolve.

Maybe there'd be yet another way for us to inject ourselves into the conversion process with an extension, in order to collect references and footnotes, but our integration with Python-Markdown is already quite complicated so I'm reluctant to complicate it further.

@pawamoy
Copy link
Contributor Author

pawamoy commented Dec 13, 2024

  1. Disable summaries. But I'm not clear on what they look like so maybe we do want them. I need more info to know.

Try serving the docs locally to see how it renders 🙂

@waylan
Copy link
Member

waylan commented Dec 13, 2024

IMO this is very clearly a bug in mkdocstrings. Requiring me to change Markdown which otherwise renders fine is unacceptable. And requiring me to use additional extensions to render Markdown which otherwise renders fine is equally unacceptable. That said, I understand that this is not an easy problem to solve, but, IMO, it is a problem that mkdocsstrigns needs to solve. Fortunately, as a workaround, I am not opposed to changing the Markdown in this instance. However, if mkdocstrings maintains its current behavior, I would expect you to receive continuing reports from users as more of them encounter this issue.

I have encountered other summary implementations before and the only behavior that works consistently is one where the entire document is rendered and then the summary is split out. Or at least that is how it needs to appear to work to the user. It is possible that an implementation could do some gymnastics under the hood to avoid needing to render the entire source document, but, however it is implemented, the end user should be able to assume that the split happens after rendering.

  1. Links are striped from summaries and only the label text is included.
    Doesn't seem possible. The non-natively-resolved reference could a valid autorefs cross-reference. In this case we do want the warning.

I'm saying that it might be preferable to some users to never have any links in summaries. Therefore, a config option to remove all links from summaries, regardless of how they get there, might be a sensible option. Of course, in this instance, it wouldn't resolve the issue because the source text is not actually recognized as a valid link.

I'd be tempted to say that this is a limitation of the summary feature, and that users should account for it rather than the opposite. The same thing happens for rendering docstring sections by the way, since docstrings written following the Google-style or Numpydoc-style will be split into sections (and sections into items), and each section (item description) will be rendered separately, meaning each section (item) must include its own references and footnotes for them to resolve.

I see the two as very different. It is easy to understand that each docstring is a standalone document. And I am assuming that in this discussion. However, for an individual document (docstring in this case) to not render correctly because only a portion of it is fed to the renderer is a bug.

In any event, the above is my personal opinion about the general behavior of mkdocstrings' summary feature. However, this is not an issue opened in the mkdocstrings project. Presumably any further discussion about mkdocstrings' behavior should happen on that project. So, going forward in this discussion, let's focus on the immediate workaround with mkdocstrings' current behavior.

I expect that we will do one of the following:

  1. Disable summaries.
  2. Reformat that one link.
  3. Rework the text so that the link doesn't appear in the summary.

Try serving the docs locally to see how it renders

That is my intention. Unfortunately, I don't have access to a local instance right now. When I do I will report back.

@pawamoy
Copy link
Contributor Author

pawamoy commented Dec 13, 2024

IMO this is very clearly a bug in mkdocstrings. Requiring me to change Markdown which otherwise renders fine is unacceptable. And requiring me to use additional extensions to render Markdown which otherwise renders fine is equally unacceptable. That said, I understand that this is not an easy problem to solve, but, IMO, it is a problem that mkdocsstrigns needs to solve. Fortunately, as a workaround, I am not opposed to changing the Markdown in this instance. However, if mkdocstrings maintains its current behavior, I would expect you to receive continuing reports from users as more of them encounter this issue.

From memory, we had 4-5 occurrences, including this one.

I see the two as very different. It is easy to understand that each docstring is a standalone document. And I am assuming that in this discussion. However, for an individual document (docstring in this case) to not render correctly because only a portion of it is fed to the renderer is a bug.

Docstrings are not meant to be rendered "as-is". When you format them following Google-style or Numpydoc-style conventions, they're not just Markdown anymore. Rendering them "as-is" is a mess, just try it and you'll see all your parameters and their descriptions rendered on a single line without any proper formatting.

The docstring styles let you structure data using plain text, so yes, you either need to pre-parse them to split everything in sections and items that you render separately, or you need to enable a Markdown extension that will understand these styles. We do the former because the latter would require communication between the Markdown extension and the tool that extracts data from Python source (Griffe here), which is even more complicated to achieve, especially when the tool itself strives to be markup-agnostic.

But yes, we can probably improve summary handling. I'll think about it. And yes this conversation should continue in mkdocstrings ☺️


I expect that we will do one of the following:

Sounds good!

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

No branches or pull requests

2 participants