Skip to content

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

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

melissawm
Copy link
Member

@melissawm melissawm commented Oct 13, 2023

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

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>
@melissawm melissawm changed the title MAINT: Update environment.yml to match requirements files DOC: Add tags infrastructure for gallery examples Oct 13, 2023
@story645 story645 added the Documentation: build building the docs label Oct 13, 2023
@jklymak
Copy link
Member

jklymak commented Oct 14, 2023

This seems good. Thanks for mocking it up. There are a numnber of suggestions for the layout etc that are probably sphinx-tags things?

  1. I wonder if the colors should be defined in the tags file somehow?

  2. It's a shame that the tag page isn't visual.

  3. The breadcrumbs don't seem to be working?

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?

@story645
Copy link
Member

I wonder if the colors should be defined in the tags file somehow?

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?

@QuLogic
Copy link
Member

QuLogic commented Oct 20, 2023

Are we able to use spaces in the tags? At least after the colon?

@melissawm
Copy link
Member Author

Yes @QuLogic - thanks to @JWCook we can now do that 😄

I will cut a release of sphinx-tags in the next day or two and will update this PR accordingly.

@melissawm
Copy link
Member Author

melissawm commented Oct 30, 2023

Alright folks - just released sphinx-tags v0.3 and will mark this as ready for review. To answer a few of the questions above:

Does this work by making a bunch of static index pages?

Yes, it does. There may be better solutions but it generates pages on build-time, so no need to store them in the repo.

Specific to the PR I wonder if a better way than Note can be used to indicate the tags index?

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?

I wonder if the colors should be defined in the tags file somehow?

Because these files are generated at build time, I'm not sure we could do that without significant rework.

It's a shame that the tag page isn't visual.

Agreed! Definitely something to consider for a future release of sphinx-tags.

wondering if we can do a [animation] tag that pulls every [animation:] sub example?

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.

@melissawm melissawm marked this pull request as ready for review October 30, 2023 19:49
@story645
Copy link
Member

Would not be able to pull this off for this release though.

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).

Comment on lines +6 to +10
--sd-color-secondary: #ee9040;
--sd-color-success: #28a745;
--sd-color-dark: #323232;
--sd-color-danger: #dc3545;
--sd-color-light: #c9c9c9;
Copy link
Member

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?

Copy link
Member

@story645 story645 left a 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.

Comment on lines 19 to 20
.. note::
You can also browse the example gallery by :ref:`tags <tagoverview>`.
Copy link
Member

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.

@melissawm
Copy link
Member Author

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.

@melissawm
Copy link
Member Author

Guess CI isn't complaining about the tag so

I'm not sure what you mean, are you having issues building?

@story645
Copy link
Member

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.

@tacaswell tacaswell added this to the v3.9.0 milestone Oct 31, 2023
* Add minimum sphinx-tags version
* Fix trailing whitespace
* Change admonition title
@melissawm
Copy link
Member Author

@tacaswell tacaswell merged commit 1335730 into matplotlib:main Oct 31, 2023
@tacaswell
Copy link
Member

Lets kick the tires on this at the pydataNYC sprint today!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: build building the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants