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

The syntax [symbol][] doesn't work as in mkdocstrings-python #16

Open
llucax opened this issue Mar 4, 2024 · 18 comments
Open

The syntax [symbol][] doesn't work as in mkdocstrings-python #16

llucax opened this issue Mar 4, 2024 · 18 comments

Comments

@llucax
Copy link

llucax commented Mar 4, 2024

I'm trying to migrate a project using mkdocstrings-python to use this handler instead, and I use the syntax [symbol][] quite a bit, but it doesn't seem to be working with this handler. In particular I use it a lot to link to Python stdlib symbols, like [asyncio][], [float][], etc.

@llucax
Copy link
Author

llucax commented Mar 4, 2024

I just noticed the syntax [symbol][^] can be used, so I wonder if this is a bug or a feature. For compatibility and to make migration of projects easier1 it would be good if both are accepted.

Footnotes

  1. , It is quite nice that, except for this issue, one can just switching to use this handler and you don't need to do anything else, you can start using relative references and migrate old absolute references progressively, if at all.

@analog-cbarber
Copy link
Member

I think that in the case of python standard library definitions like asyncio and float, those don't seem to appear using the lookup mechanism used for early reference checking, but they do show up in the documentation. I think this comes up with a few other libraries that are missing entries from their objects.inv file.

The question is whether the problem is only that you get missing reference errors from the handler but the cross refs still work in the actual documentation. If that is the case, you can workaround this by prepending ? to the cross-reference pattern, which tells the handler to not check the cross reference.

See https://analog-garage.github.io/mkdocstrings-python-xref/1.6.0/#cross-reference-checking

The current implementation of this feature can produce false errors for definitions from the python standard library. You can disable the check on a case-by-case basis by prefixing the reference expression with a ?, for example:

This function returns a [Path][?pathlib.] instance.

@analog-cbarber
Copy link
Member

The intent is for all existing cross-references to work the same way as before. So apart from the cross reference checking issue, [foo][] will be the same in this handler and the original. In your example, what is symbol? Is that in your own code?

@llucax
Copy link
Author

llucax commented Mar 4, 2024

I have strict mode enabled by default, so it errors and stop generating/serving the docs.

I disable it and it seems like the link works properly. I also noticed this only fails in class docstrings, for module docstrings it works fine.

I would prefer to avoid the syntax [Path][?pathlib.] as it makes the symbol much harder to read in text mode (I would just copy the full path manually in that case, like [Path][pathlib.Path], it is much clear and the other syntax only saves 3 characters).

It would be very nice if this could somehow work as in the upstream handler, couldn't you just add a special case for the empty string and fall back to the upstream handler when the empty string is used as target?

@analog-cbarber
Copy link
Member

The issue is that the upstream handler doesn't check the cross reference when the source is processed. It only checks it later after the source information has been lost which means you get errors saying "can't find reference to 'foo'", which can then require you to search your code for foo.

Clearly, there is something different about how the symbol is looked up later. I think that using the full path is perfectly fine but I think there may be some cases where the full path does not work either.

And as I said, you will find some libraries where the objects.inv file is not complete (I think numpy might be one of those) so you might not want to use strict checking in any case.

@analog-cbarber
Copy link
Member

I suppose I could add a special option to disable cross-reference checking for references not translated by this handler but then you would lose the benefit of the source location for error reporting.

@llucax
Copy link
Author

llucax commented Mar 6, 2024

OK, I see. About:

And as I said, you will find some libraries where the objects.inv file is not complete (I think numpy might be one of those) so you might not want to use strict checking in any case.

In that case I think it is fair to require the ? syntax or disabling the check. But for complete objects.inv ideally things should just work.

What it is not clear to me is if this extension is providing extra/better checking of cross referencing than the upstream project, so if I disable the checking I will get the exact same checks as the upstream project provides, or will I get no checks at all and might end up with broken links?

@analog-cbarber
Copy link
Member

What checking in this handler gives you is the source location of the offending pattern. Using the standard handler, the checking is not done using Griffe but by the generic mkdocstrings renderer which has no access to the source location of the original cross reference specification. So you end up having to grep your code to try to find what is causing the lookup error. Sometimes that can be non trivial.

@llucax
Copy link
Author

llucax commented Mar 7, 2024

Sorry to ask again. I understand that the checker in this extension is more powerful and precise than the upstream checking (with the downside of having this issue), but it is till unclear what happens if I set check_crossrefs: false. Will I still get the same checks as if I were using the upstream mkdocstrings-python?

@analog-cbarber
Copy link
Member

The checks done in this handler are in addition to any that are done upstream so when enabled you can get two errors one with source info and one later on without. If you turn this off, then you only get the upstream errors.

@llucax
Copy link
Author

llucax commented Mar 28, 2024

OK, thanks. I gave this another try with the early checking disabled and it is true that it is quite nice to have the exact file:line of the error. Also this seems to be false:

I disable it and it seems like the link works properly. I also noticed this only fails in class docstrings, for module docstrings it works fine.

It seems it have worked with [asyncio][^] only because this particular module is importing asyncio, so I guess it is finding it as ^ resolves to the top-level. This is why it fails in a class, because then [^] is the module instead of the top-level.

So I wonder if there is a way to add some sort of [asyncio][/] that will look up in the top-level. I guess not, because then I guess the empty string ([]) can be an alias for [/] and then everything should work.

@llucax
Copy link
Author

llucax commented Mar 28, 2024

For now the only consistent solution I can find is either using check_crossrefs: false and lose the early/precise checking, or repeating the top-level built-in references (like [asyncio][asyncio]).

@llucax
Copy link
Author

llucax commented Mar 28, 2024

Another difference with the upstream handler, even when using check_crossrefs: false, [`asyncio`][] produces a broken link, while it works fine with the upstream handler.

@llucax
Copy link
Author

llucax commented Mar 28, 2024

Also this trick to hook the macros plugin to mkdocstrings doesn't work:

But maybe it just needs some adaptation to hook it to this plugin instead. In any case, I think sadly it's too many road bumps for me, so we'll probably need to stick to the upstream handler. I really hope we can eventually switch because using fully qualified names is just too noisy.

@analog-cbarber
Copy link
Member

You should be able to just use the standard [asyncio][] for top level references. If you want to refer to a symbol that is in the top-level of the same module you can write [foo][(m).].

I wouldn't be surprised if using markup inside the title might not be handled correctly by my code, so if you are really writing:

[`asyncio`][]

that might not work. I will look into that and file a separate issue.

I don't think I really understand your issue with mkdocs-macros.

@llucax
Copy link
Author

llucax commented Apr 2, 2024

I will look into that and file a separate issue.

Thanks!

I don't think I really understand your issue with mkdocs-macros.

This is the script I'm using to hook with mkdocs-macros:

https://github.com/frequenz-floss/frequenz-channels-python/blob/d9b7d224b7418cefcbe58d130928d7f0178b065a/docs/_scripts/macros.py

In particular when using the docstring_summary macro defined there, I get this output instead of the expected excerpt:

image

While when using the original mkdocstrings plug-in it renders like this:

image

Again, I didn't looked into it in details, so maybe I'm just missing updating some part of the script or configuration.

@llucax
Copy link
Author

llucax commented Apr 2, 2024

You should be able to just use the standard [asyncio][] for top level references. If you want to refer to a symbol that is in the top-level of the same module you can write [foo][(m).].

Yes, [asyncio][] works with check_crossrefs: false or by disabling strict mode in mkdocs. Otherwise it produces a spurious error that stops the generation when both check_crossrefs and strict are enabled.

@analog-cbarber
Copy link
Member

Thanks. I am not familiar with mkdocs-macros. I will have to spend some time playing with that, but I am not sure when I will have the time...

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