Support cross-refs in attributes signatures / Support stringified types in base classes subscripts

[llucax]

Describe the bug

Nested classes attribute types don't get properly linked.

To Reproduce

I hope it is enough to show an open source repo with the issue.

• config: https://github.com/frequenz-floss/frequenz-channels-python/blob/v0.x.x/mkdocs.yml
• source: https://github.com/frequenz-floss/frequenz-channels-python/blob/a4b873ba8c3b38df78ce8f6883dc82d143894859/src/frequenz/channels/util/_file_watcher.py#L21-L38
• rendered docs: https://frequenz-floss.github.io/frequenz-channels-python/v0.16/reference/frequenz/channels/util/#frequenz.channels.util._file_watcher.FileWatcher.Event

Expected behavior

The Event attribute types get properly linked.

Screenshots

---

Even for pathlib.Path that is a Python standard library, for which the cross-linking is properly configured doesn't work in the nested class. But in other parts it works perfectly.

Information (please complete the following information):

• OS: Linux
• Browser: firefox
• mkdocstrings: 0.22.0
• mkdocstrings-python: 1.1.2

[llucax]

Related but probably different issue, it doesn't get linked either as a generic type instance:

But I'm not sure if that is because the type is specified as a string, or because it is a type defined inside the class itself (yeah, probably this is not the cleanest pattern, but if Python can deal with it maybe mkdocstrings should too?).

Let me know if I should create a separate issue for this.

[pawamoy]

Hello, thanks for the report. Yes, a public repo is perfect, thank you!

However I don't think this is a bug, but just a current limitation. If you look at this heading https://frequenz-floss.github.io/frequenz-channels-python/v0.16/reference/frequenz/channels/util/#frequenz.channels.util._event.Event.is_set, you'll see that bool isn't transformed into a link either. That's because currently we don't write cross-refs in attributes types. There's no reason not to do it, it's just that I didn't think about it previously, and didn't get time to work on it yet 🙂

I'll keep this issue open as a reminder!

[pawamoy]

About Receiver['FileWatcher.Event'], do you use a string annotation in your source code too? Do you import annotations from __future__ in that module? EDIT: yes and yes, I checked the source. OK, this one needs investigation.

A bit of explanation: since you import future annotations, Griffe concludes that you don't need to use stringified annotations since all annotations are already postponed. However in your case you're inheriting from a class which uses a non-defined yet type as subscript. So you don't have a choice but to use a string. And Griffe does not understand that. I'll see if there's something we can do to improve support for such a scenario.

In the meantime, yes, you can probably avoid nesting classes in such a cyclic way.

[llucax]

Hi @pawamoy, thanks for the quick reply!

However I don't think this is a bug, but just a current limitation. If you look at this heading frequenz-floss.github.io/frequenz-channels-python/v0.16/reference/frequenz/channels/util/#frequenz.channels.util._event.Event.is_set, you'll see that bool isn't transformed into a link either. That's because currently we don't write cross-refs in attributes types. There's no reason not to do it, it's just that I didn't think about it previously, and didn't get time to work on it yet slightly_smiling_face

Yeah, after I write the issue I noticed there were quite a few other places where links were missing. Thanks for the clarification!

A bit of explanation: since you import future annotations, Griffe concludes that you don't need to use stringified annotations since all annotations are already postponed. However in your case you're inheriting from a class which uses a non-defined yet type as subscript. So you don't have a choice but to use a string. And Griffe does not understand that. I'll see if there's something we can do to improve support for such a scenario.

I see, thanks. Yes, I know is not a very happy thing to do, so I totally understand if it is not worth investing the effort on supporting this, but since Python does support it, maybe you still want to support it too :)

[pawamoy]

The issue with "Python supports it" is that Python supports a lot of things 🤣 Being a very dynamic language, we will never be able to statically support everything it does. Some examples are wildcard imports (PITA), dynamic base classes, and really anything that modifies attributes/functions/classes with several levels of indirection.

Besides, here it's more about what mypy supports rather than Python itself. And we will definitely never be as clever as mypy regarding type inference and semantics.

For these difficult cases, there's always the option to fall back on introspection, but it has a cost, and it cannot even ensure better results (especially regarding type annotations).

In the end, what we must do on Griffe's side it to properly document these limitations 🙂 I'll add a doc label to this issue 😄

[llucax]

Besides, here it's more about what mypy supports rather than Python itself. And we will definitely never be as clever as mypy regarding type inference and semantics.

Yeah, I mainly meant mypy supports it, which should be reasonable set of features that work mostly statically, but I understand even then mypy has a lot of special cases to support built-ins, etc. So I know it is a pretty ambitious goal to cover that subset too :)

In the end, what we must do on Griffe's side it to properly document these limitations slightly_smiling_face I'll add a doc label to this issue smile

Yeah, this makes sense too.

We are trying to find out a subset of Python that we can use and play well with the different tools we use, which is not trivial. 👍

[matankley]

Hey! Thanks for the detailed explanation in this thread. I came across it while searching how to fix the limitation that in attribute there are no cross-refs.

I face the same issue that you discussed above. Is it planned to be prioritized ?

[pawamoy]

Sure, thanks for the reminder. Right now I'm working on preserving the paths of members through aliases, and I'll work on this next 🙂

[pawamoy]

Done in mkdocstrings-python insiders v1.6.0.1.4.0 🙂

[matankley]

@pawamoy Amazing!!! thank you!