Skip to content

ENH: Add dark mode support for some pydata packages#723

Merged
choldgraf merged 20 commits intopydata:mainfrom
choldgraf:pydata
Jun 16, 2022
Merged

ENH: Add dark mode support for some pydata packages#723
choldgraf merged 20 commits intopydata:mainfrom
choldgraf:pydata

Conversation

@choldgraf
Copy link
Copy Markdown
Collaborator

@choldgraf choldgraf commented Jun 11, 2022

This does two things:

  1. Adds a page where we can show off the outputs from various libraries in the PyData ecosystem.
  2. Adds a few more CSS rules to make sure that these libraries look reasonable in the dark mode.

As part of that, it also adds a (documentation only) dependency on myst-nb, and uses this execute our new pydata demo page. That should make it easier to cache the execution of that page so that it doesn't have to re-execute every time.

I think there's still some improvement to make (e.g., in the xarray CSS styling) but hopefully this makes it easy to naturally extend the CSS as new edge-cases are found with these libraries.

Also fixes two bugs:

  • Primary sidebar TOC was unnecessarily indented
  • Primary sidebar scrollbar was not colored properly (the variables didn't seem to be inherited so this set the color as the chrome default)

Check out the demo docs here: https://pydata-sphinx-theme--723.org.readthedocs.build/en/723/demo/pydata.html

@choldgraf choldgraf marked this pull request as draft June 12, 2022 07:46
}

div.cell_output tbody tr:nth-child(odd) {
background: #f5f5f5;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure about a hard coded color ?

@12rambau
Copy link
Copy Markdown
Collaborator

you also moved the search item right ?

@12rambau
Copy link
Copy Markdown
Collaborator

12rambau commented Jun 12, 2022

sorry for making a commit I didn't know how to make a suggestion. Now I do.
...and I didn't see it was a draft.I should stop making review on sunday morning before my first coffee,I'm always making mistakes

}

// customize xarray display
.xr-wrap {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.xr-wrap {
.xr-wrap .x-var-list {

@choldgraf
Copy link
Copy Markdown
Collaborator Author

choldgraf commented Jun 12, 2022

Oops I did not see that you had left some comments, sorry about that! I actually decided to take a slightly different approach here. Rather than special-casing each library, I decided to special case the execution / notebook libraries that people often use. So this PR now adds some rough dark theme support for jupyter-sphinx and myst-nb. I think that this will be a more sustainable solution than trying to special-case every visualization library out there. Does that make sense?

Also sorry about the search bar stuff, that was just some accidental cruft from another experiment!

I think it's ready for a look now - the failing test is because MyST-NB pins the upper bound of Python to 3.10

@choldgraf choldgraf marked this pull request as ready for review June 12, 2022 09:43
@choldgraf
Copy link
Copy Markdown
Collaborator Author

cc @tupui - I'm not sure what tool you're using to generate plots/outputs/etc in the scipy docs. This PR adds support for MyST NB and jupyter-sphinx, but we can add another if there's a different one needed to test (as long as it doesn't conflict w/ the previous two tools)

@tupui
Copy link
Copy Markdown
Contributor

tupui commented Jun 12, 2022

cc @tupui - I'm not sure what tool you're using to generate plots/outputs/etc in the scipy docs. This PR adds support for MyST NB and jupyter-sphinx, but we can add another if there's a different one needed to test (as long as it doesn't conflict w/ the previous two tools)

On SciPy we are using matplotlib.sphinxext.plot_directive. We have very simple needs.

@choldgraf
Copy link
Copy Markdown
Collaborator Author

OK added in a simple example w/ the plot directive in matplotlib...it seems to look OK-enough to me 🤷

@tupui
Copy link
Copy Markdown
Contributor

tupui commented Jun 12, 2022

I will have a look tmr thanks 😊

Copy link
Copy Markdown
Contributor

@tupui tupui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @choldgraf.

For the figures it's a matter of taste. I will keep the inversion in SciPy as I think it looks better because I can blend it with the background.

The code cells look a bit boxy here with the thin white border which is not present with the light theme.

Maybe consider adding a mathjax example? Or just some CSS for it. It's a "heavy" dependency if just for one example.

Comment thread docs/demo/pydata.md Outdated
Comment thread docs/demo/pydata.md Outdated
Comment thread docs/demo/pydata.md Outdated
Comment thread docs/contribute/topics.md
We store our PyData-specific SCSS in two relevant files, both in the `src/pydata_sphinx_theme/assets/styles/` folder:

- `extensions/_execution.scss` - styles for Sphinx libraries that execute and insert code into the documentation. For example, MyST-NB, Jupyter Sphinx, and the Matplotlib `plot` directive. Most PyData support should go here via generic improvements that all packages benefit from.
- `extensions/_pydata.scss` - styles for specific libraries in the PyData ecosystem. In general we should try to keep this minimal because it is mostly special-casing single library quirks.
Copy link
Copy Markdown
Contributor

@tupui tupui Jun 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean we should contribute specific changes we have in SciPy for instance? Is this like a showcase section or it would be used somehow?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption is that there might be many different communities using the functionality of a given PyData package, so if there's a SCSS change that improves something in this category, we should put it in this theme (in the _pydata.scss) file so others benefit from it. See the example in there, it is for an xarray output bug that was conflicting with Bootstrap

Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>
@choldgraf
Copy link
Copy Markdown
Collaborator Author

choldgraf commented Jun 13, 2022

Yeah I agree on the boxiness of the cells (especially with the Jupyter Sphinx extension...I think it looks pretty fine w/ the MyST outputs).

re: inversion, my main concern was that this is changing all of the colors in the plot, which I imagine might have negative consequences for the reader's perception of the plot.

re: MathJax example, what kinds of things did you have in mind? I just added a docs section in our special theme elements for math, and this also caught a couple of math bugs :-)

@tupui
Copy link
Copy Markdown
Contributor

tupui commented Jun 13, 2022

For the maths I noticed that the color did not match. It was light gray so not very readable. I added:

html[data-theme=dark] .MathJax_SVG *  {
    fill: var(--pst-color-text-base);
}

@choldgraf
Copy link
Copy Markdown
Collaborator Author

ah good call - we weren't testing SVG math output here (we're using the HTML version). Latest commit adds that rule 👍

@choldgraf
Copy link
Copy Markdown
Collaborator Author

I think that this one is now ready to go, anything else to add here?

Comment thread .github/workflows/tests.yml
Copy link
Copy Markdown
Contributor

@tupui tupui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@drammock
Copy link
Copy Markdown
Member

LGTM too

Comment thread docs/contribute/topics.md
@choldgraf
Copy link
Copy Markdown
Collaborator Author

OK merging this one in since we have a bunch of approvals!

@choldgraf choldgraf changed the title Add dark mode support for some pydata packages ENH: Add dark mode support for some pydata packages Jun 16, 2022
@choldgraf choldgraf merged commit 4c16582 into pydata:main Jun 16, 2022
@choldgraf choldgraf deleted the pydata branch June 16, 2022 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants