-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
feature: extension(?): relative cross-references in docstrings, but resolved by the handler/extractor #166
Comments
Thanks for this detailed feature request @the-13th-letter 🙂 I appreciate the thoroughness and the honest opinions. Before answering, let me try and explain how cross-references work.
Now for my answer. If we wanted to implement your suggestion, we would have to either:
The second option doesn't look good to me, because:
So that leaves us with option 1. I have read mkdocstrings/autorefs#37 again and came to the conclusion that it's my preferred solution going forward. I'll expand there on the general debate over generic vs handler-specific handling of references, and stay focused here on the syntax you propose. While I agree that using the current object's scope to resolve partial paths (like First, the restrictions it brings. If we are to rely on the object's scope to resolve a partial path, that means the object we reference must actually exist in the given scope (either at runtime or just in the sources, guarded by Second, and as you mentioned, there could be cases where the reference becomes ambiguous, because it's a complete path but this path also exists in the current scope. For example: from official import thing
import third_party as official
def func():
"""Link to [official][] package."""" Here it's unclear whether The point of readability is important, but IMO the most readable references will always be the fully typed ones. Relative references, whether they use the current scope (not visible when displaying |
I'll need a bit more time to answer on mkdocstrings/autorefs#37, but the crux of it, so you know where I'm headed to, is the following:
What this means is that the handler/extractor could then decide how to compute metadata, and what inputs it supports, meaning that your suggestion could actually be supported 🙂 For Python specifically, Griffe would probably be responsible for this metadata object, and since it supports extension, I can imagine an extension for your syntax suggestion, and other ones for the other suggested syntaxes. Additionally, autorefs itself could propose a generic syntax, that handlers/extractors could choose to support or not, paving the way to a unified way of writing (relative/cross-language) references. |
OK, got it.
I'm not particularly hung up on implementing this in Griffe instead of in
I'm inclined to say that having to actually import an object (or its containing module) to reference it in a short manner is a feature of this system. Or rather, if your actual code uses this name so little that you don't care enough to import it, then it's also very probably not important enough for you to want to document it using only a short local name. And of course, there's always the option of overriding your linter/formatter in this case to keep the name. Or file this as a bug/wishlist feature for the linter/formatter to detect. I understand that it is currently treated as a bug on the author's side and not on the tool's side… but I don't think that that's actually true, or that it would necessarily stay this way in the long run. Also, I'm totally fine with this being available only as an extension/a plugin that users need to explicitly opt in to.
Yeah, well, that's just asking for trouble, and You Really Shouldn't Do That™. And if you actually Really Do That, then woe be to you for writing such horribly readable code. In this particular case I would probably link to the renamed There's only one scenario I can think of where I would tolerate this kind of thing: (Also, during my last attempt at this type of monkey patching otherwise missing functionality—providing a poor substitute for On a related note, I would be fine with this being available only as an extension/a plugin that users need to explicitly opt in to.
Clearest, yes. Most readable, I dunno; I think that depends a lot on how unwieldy the full path is, and even if the package is known under a standard alternate name such as In the abovementioned case, yes, the (full) scope is not visible to the reader of the docstring, and it would be prudent of the author of this docstring to not relative-reference names that can't be clearly matched to enclosing scopes or parent modules. But I'm a strong believer of treating the user as And concerning the “dot-prefix“ syntax, I don't believe that the ability to be able to decode what the prefix refers to is that much consolation. For both systems, if you use them where you ought to better have used an absolute cross-reference, you'll probably be bogged down by trying to decode just what in the world that crossref is referencing. If they are both used where they make sense, then the “scope“ version probably needs less lexical parsing than the “dot-prefix“ version. I imagine that you could get used to both of them, given enough exposure. That said, from my own experience with (not) using Python's relative import syntax, I'm pretty sure that I would need to do calculations and double-checks to make sure my identifiers are correct in the “dot-prefix“ version. I really can't imagine anyone needing that for the “scoped” version but not needing it for the “dot-prefix” version. … Again, I realize that I am asking for a certain implementation that only works for some kinds of relative cross-references, that only works correctly4 for some kinds of relative crossrefs, and that there is a lot of potential both for ambiguity in what the user actually meant and for the reference texts to be non-sensical to readers consulting the raw docstring without having access to the rest of the source code (e.g. … Also, btw, did I perhaps ever mention that I'm completely fine with this being available only as an extension/a plugin that users need to explicitly opt in to…? Footnotes
|
If I understand correctly, you're fine with this being available only as an extension... 😀 Good then, this is likely what will happen 🙂
Just a quick maintainer perspective: there can be a lot of less-advanced users that will trip over anything that can be tripped over. That generally means more user support falling down onto maintainers (helping them, turning down feature requests, closing invalid bugs, writing non-confusing documentation, putting more efforts into warning messages and error handling). That's something to take into account. |
Also, thanks again for your suggestion. Using the object's scope is something I didn't think about before and it's a nice idea 😄 |
Thanks :)
Surely. And maybe my view on this will change once I have done some (serious) software maintenance work. But as of now, I am shaped much more by my repeated experiences at |
I'll move this to mkdocstrings-python, as I don't think there's much to do in Griffe itself. |
Released as v1.9.0 of the insiders version 🙂 (v1.11.1.1.9.0) |
Now that (some) versions of `mkdocstrings-python` have support for [scoped crossreferences] [SCOPED], references to documented objects can be named like they would be in code. This leads to (Markdown) docstrings that are much more readable in source form than when having to type or re-type the fully qualified identifier name. [SCOPED]: mkdocstrings/python#166
Is your feature request related to a problem? Please describe.
I find myself repeatedly writing (
mkdocs
/mkdocstrings
-formatted) documentation of the following form, where I have to reference other functions doing things that affect my current function:i.e. where the link to
myawesome.package.configuration.get_config
needs to be spelled out in full. Generally this happens when things likeenum.Enum
s andtyping.NamedTuple
s come into play, and these get moved to separate packages but constantly must be referenced.Describe the solution you'd like
mkdocstrings
, the main benefactor of Griffe, and its parent systemmkdocs
, use Markdown, which by design is intended to be readable in its source form. Python comes with docstring support and a built-inhelp
function, i.e. a place for documentation to be placed and a system to query it, so that it is easy to read and write documentation in its source form. My point being, the source form of the docstring should be readable.Therefore, in (my best interpretation of) the spirit of Google's style guide and of Markdown's design rationale, I imagine the cleanest way to specify this cross-reference would be:
i.e. that cross-references can be specified using local object names that the Python environment already knows about (here: the
myawesome.package.configuration
module via theconfiguration
alias). This is very readable even with Python'shelp
function (and inmkdocs
-generated documentation as well, of course), and it is intuitively clear for any Python programmer how to change this cross-reference if a different object needs to be referenced, or if anything has been renamed or imported under a different name or whatnot. It is also based on information that Griffe probably already extracted when resolving type hints or the like, so Griffe (and thusmkdocs
) could likely easily be taught to “understand“ such links.When using this notation, there would only be a need to format cross-references of the form
[`identifier`][]
or[`dotted.path`][]
, i.e. where the link text and the link target are equal, and are both valid Python identifiers. I imagine that this is unambiguous enough to have a very low false-positive rate, so it should be “relatively safe“ to treat every such equal-text-and-target link which is also a valid Python identifier as a cross-reference to resolve this way.From my understanding of the architecture of
mkdocs
+mkdocs-autorefs
+mkdocstrings-python
, I believe there are two parts to this proposal:Griffe (or a Griffe extension) should check if the current docstring contains a Markdown cross-reference of the aforementioned type. If we detect such a reference, Griffe should either resolve it directly (using the names from the module it already knows about) or else add enough information (e.g. names of possible candidates) to some auxiliary structure to help resolve it later.
mkdocs-autorefs
(or an extension) should resolve the unresolved link target using the auxiliary structure, if it hasn't been resolved yet.Describe alternatives you've considered
I'm aware of #27, #28 and of analog-garage/mkdocstrings-python-xref. I can't speak to the quality of the code, but I find the proposed syntax for relative identifiers especially repulsive, in a this-is-no-longer-text-this-is-code sort of way. For me this is no better for reading or writing than using full package names.
I'm aware of mkdocstrings/autorefs#37. Again, I really detest the proposed syntax, as it goes against the spirit of having documentation readable and understandable even in source form.1 I also disagree with the notion that the current object
.
in a relative link should be the class/function/whatever that the docstring belongs to, because relative import syntax only considers packages and modules, and nested scopes use modifiers likenonlocal
orglobal
but otherwise unqualified identifiers. I expect forcing users to use unestablished-but-superficially-familiar syntax for actually-familiar things would also force them to constantly have to look up the syntax just to get it right, because they can't quickly tell if a..ParentClass
link inSubClass.method
refers to the sibling classParentClass
the same module, or in the parent module, or even if the superclass isParentClass
itself.(I'm also aware of @pawamoy's, @lmmx's and @oprypin's comments and viewpoints there. I'm hoping you will chime in on this proposal too, if only because I think the new limited scope and clear division of (software) responsibility could simplify that pull request quite significantly.)
(Also, I can't tell if the pull request is dead, or just on hold, and if your stances on that design have changed since then.)
Footnotes
No, I don't find Python's relative import syntax really aesthetical either, and would not want it in user-facing non-code text. For what it's worth, Google's style guide also explicitly discourages its use, though for different reasons. ↩
The text was updated successfully, but these errors were encountered: