-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Support cross-refs in attributes signatures / Support stringified types in base classes subscripts #586
Comments
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. |
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 I'll keep this issue open as a reminder! |
About 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. |
Hi @pawamoy, thanks for the quick reply!
Yeah, after I write the issue I noticed there were quite a few other places where links were missing. Thanks for the clarification!
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 :) |
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 😄 |
Yeah, I mainly meant
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. 👍 |
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 ? |
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 🙂 |
Done in mkdocstrings-python insiders v1.6.0.1.4.0 🙂 |
@pawamoy Amazing!!! thank you! |
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.
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):
mkdocstrings
: 0.22.0mkdocstrings-python
: 1.1.2The text was updated successfully, but these errors were encountered: