-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC: Add tags infrastructure for gallery examples #27083
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
Conversation
Adds sphinx-tags as a dependency and some basic configuration to display tags with color, using sphinx-design. Co-authored-by: hannah <story645@gmail.com> Co-authored-by: esibinga <vegheadeva@gmail.com>
This seems good. Thanks for mocking it up. There are a numnber of suggestions for the layout etc that are probably sphinx-tags things?
Does this work by making a bunch of static index pages? Specific to the PR I wonder if a better way than Note can be used to indicate the tags index? |
Maybe by using the tags directly in the tags file-> which also means linking on them would get all the related images? If that's too cluttered, maybe just the [parent category] tag as a header to that section? Which as an aside, was thinking of the convo yesterday and @esibinga making the good point that animation is itself a good tag and wondering if we can do a [animation] tag that pulls every [animation:] sub example? |
Are we able to use spaces in the tags? At least after the colon? |
Alright folks - just released sphinx-tags v0.3 and will mark this as ready for review. To answer a few of the questions above:
Yes, it does. There may be better solutions but it generates pages on build-time, so no need to store them in the repo.
I am happy to move this to somewhere else - my thinking was because this is still a prototype we may not want to make this a huge feature while the tags are not at least 50-70% done?
Because these files are generated at build time, I'm not sure we could do that without significant rework.
Agreed! Definitely something to consider for a future release of sphinx-tags.
Also something we're tracking on sphinx-tags. Would not be able to pull this off for this release though. @story645 since it would be useful to have this for the sprint at PyData NYC tomorrow, I'll be prioritizing any further comments here. For now, I'm marking as ready for review. |
Yeah of course! Sorry I didn't make that clear. My major priority is that folks can add a ..tag directive and not have the build break, which this tag should probably be added to the rstcheck in pyproj.toml (my guess is what I'm doing wrong there is I need to add all the extension modules to the required list, but that's out of scope here). |
--sd-color-secondary: #ee9040; | ||
--sd-color-success: #28a745; | ||
--sd-color-dark: #323232; | ||
--sd-color-danger: #dc3545; | ||
--sd-color-light: #c9c9c9; |
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.
Where do these colours come from? Are there not any variable in pydata-sphinx-theme that correspond to them?
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.
Guess CI isn't complaining about the tag so 🤷♀️
Elliott's review though is making me think we may want space insensitivity so that component:axis and component : axis resolve to the same tag. At least until we can write a linter to force one or the other.
I don't consider it blocking for tomorrow though cause it's gonna be easy enough to normalize in review/post.
galleries/examples/README.txt
Outdated
.. note:: | ||
You can also browse the example gallery by :ref:`tags <tagoverview>`. |
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.
Can we name this admonition "tagging!" And then add a link to the tagging guidelines. I'm basically thinking that once yours and Eva's PRs are merged, this box would also be a good place to link to the tagging guidelines & encourage folks to tag, at least until a good chunk of the gallery is processed.
Good point on the normalizing. And yes, colors can be pulled from variables, I set them manually as a proof of concept. I can try and make them more general tomorrow, although I wasn't even suggesting these should be the final colors, these were more to give the idea of how they could look. We can think more deeply about these colors and accessibility, for example. I'm on mobile right now but can take a look at these things first in the morning. |
I'm not sure what you mean, are you having issues building? |
Other way around. I expected the rstchecker pre-commit hooks to complain about the tag directive but it's not complaining about it. It means one less thing to do for this PR. |
* Add minimum sphinx-tags version * Fix trailing whitespace * Change admonition title
6b0c68f
to
cd34b80
Compare
I am not sure what's the deal with appveyor, but the docs are building ok: |
Lets kick the tires on this at the pydataNYC sprint today! |
PR summary
Adds sphinx-tags as a dependency and some basic configuration to display tags with color, using sphinx-design.
This is a proof-of-concept implementation of tags for the gallery of examples. The actual tags will be the result of the Google Season of Docs project by @esibinga.
This PR is mostly a demo of how this would work in practice, and feedback is appreciated.
Related: #26851
There is no release note but this may need one when it is ready to merge.
cc @story645
Previews:
PR checklist