Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
DOC: Add role for custom informal types like color#27200
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The role is intended to be used for type references indocstrings like```Parameters---------- color : :mpltype:`color````It is easily extendable to other types.The naming `mpltype` was a quick choice. I'm open to better names.This PR contains one example usage in `widgets.Button` so that one cansee the effect in the built docs. Systematic applicationthroughout the codebase should be deferred to a separate PR.Closesmatplotlib#24859.Formalizesmatplotlib#27164.
Hatch is the other one I'm thinking of immediately, but also wondering if there's a way to autogenerate the ones that are a literal - either from typing stubs (where we define the literal list as a subtype) or the validators or something. |
I wouldn't go for autogeneration. You would have to specify a translation from the type to the link target (e.g.type ->type_def, which would currently fail because we mapcolor ->colors_def). Also, that's too much magic and implicitness. There's basically no effort in manually maintaining the |
Hmm, wondering if this could/should be like the where we maintain one page that's the list of types. We've talked about this before as a#9967 |
Possibly, but that's rather orthogonal to the ref implementation. That page would basically define the labels. |
ksunden commentedOct 26, 2023 • edited by story645
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by story645
Uh oh!
There was an error while loading.Please reload this page.
I will state that we now do have ColorType somewhat defined (we could add a docstring to it which links to the color reference itself, though it doesn't have one yet) https://matplotlib.org/stable/api/typing_api.html#matplotlib.typing.ColorType |
story645 commentedOct 26, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sorry Kyle I edited your comment! Tried to put it back, really wish there was a way to lock comments :/ That's probably the page that would make sense as the basis for what I'm after (+- that I wish stuff like colortype wasn't replaced by a tuple) but I also won't hold up this PR for something like that. I'm just thinking through/concerned about us ending up w/ two approaches for how we define our types in docs:
and my concern is we'll end up w/ a very confusing mix of both if we don't do any planning. |
If we are going to have a custom role that looks up an index like this, it would be good to have three or four examples of things that should be indexed, otherwise, it seems just as easy to reference the appropriate page. |
color, hatch, markerstyles, line styles, capstyles,, join styles, really probably any of the aesthetic properties that aren't a float and even then, this would be a good way to document something like the line width units. |
Yes - my suggestion was that it would be good to have at least some of that index in place. |
timhoffm commentedOct 26, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I don't think this needs to wait for deciding on a definitive index. This already improves on the current situation in two ways:
If we later decide to change the source formatting of the link to something other than |
The index approach proposes using the same directive, just defining it initially on one page. I'm not opposed to a staged start here then move - I just don't want a half/half. |
I mostly care about having reasonably formatted link that goes somewhere informative, because that affects usability. You are free to order and design the targets of the links. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is definitely going to be an improvement over status quo and switching over to a definitional role down the line shouldn't be hard. My only :/ is having the list definition in the function like that cause I can see that becoming a pain as it grows.
timhoffm commentedNov 2, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I don't think we need to be too worried about this. I expect this to grow to not more than a handful of types, so needing to change the list should be rare. |
Friendly ping. This needs a second reviewer. |
Uh oh!
There was an error while loading.Please reload this page.
The role is intended to be used for type references in docstrings like
It is easily extendable to other types.
The naming
:mpltype:
was a quick choice. I'm open to other names. (Note that:type:
seem to be available, but I'm inclinded to say it's too generic: People will find it in the code and have a hard time figuring out where it comes from.)This PR contains one example usage in
widgets.Button
so that one can see the effect in the built docs. Systematic application throughout the codebase should be deferred to a separate PR.Closes#24859.
Formalizes#27164.