Skip to content
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

Render PDEPs using myst-parser and the sphinx doc theme #51467

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Feb 17, 2023

Quick experiment for the discussion in #51461

  • Individual PDEP pages are built with sphinx
  • I injected a top navbar in the template mimicking the website's navbar, but for now just with a link back to the previous page. I think it should be possible to let it have the same items/dropdowns as the other pages as well.

Some links to compare current rendering with the one from this PR:

@@ -0,0 +1,20 @@
# Minimal makefile for Sphinx documentation
Copy link
Member Author

Choose a reason for hiding this comment

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

Those Makefile and make.bat files are not actually needed, but we could keep them in case you locally just want to build the PDEPs (although then you can also run sphinx-build -M html . _build, so that might not be worth it)

@@ -363,6 +363,8 @@ def get_source_files(source_path: str) -> typing.Generator[str, None, None]:
Generate the list of files present in the source directory.
"""
for root, dirs, fnames in os.walk(source_path):
if root.startswith("pandas/pdeps"):
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be generalized by allowing a directory in the ignore field in config.yml

@datapythonista
Copy link
Member

This is the preview of the changes here: https://pandas.pydata.org/preview/51467/pdeps/0001-purpose-and-guidelines.html

To me the benefits of this surely are not worth the extra complexity and inconsistency of UI of the PDEP pages. Can't really understand why you think this is a good idea. But if others also think it's a good idea, no big objection from my side.

@lithomas1
Copy link
Member

lithomas1 commented Feb 20, 2023

Nice, thanks for doing this @jorisvandenbossche. Having the pydata-sphinx-theme styled code blocks is definitely very nice.

Is there a reason that we're only doing this for PDEPs? It would be nice to have the full website rendered using sphinx.
(Would also make the website consistent with the docs in terms of style).

@datapythonista
Copy link
Member

Is there a reason that we're only doing this for PDEPs? It would be nice to have the full website rendered using sphinx. (Would also make the website consistent with the docs in terms of style).

That would make more sense to me than using two different systems for the website like in this PR.

The website parses the yaml config file and fetches data like the blog, releases... But those could be moved to sphinx extensions I guess.

I personally think it'd be better to put effort in providing an integrated experience of the web and the docs than migrating the whole website for some style changes we can easily achieve with the current website. But the final result seems to make more sense than the PDED hack, and if we implement the sphinx extensions in a third party package, those could be reused by other projects and remove a bit of complexity from our codebase.

@simonjayhawkins simonjayhawkins added the Web pandas website label Feb 22, 2023
@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Mar 25, 2023
@mroeschke
Copy link
Member

Closing to clear the queue, but feel free to reopen when you have time to circle back

@mroeschke
Copy link
Member

Is this still moving forward?

@jorisvandenbossche
Copy link
Member Author

I would personally like to do that, yes, as I think it is a significant improvement in readability of the rendered PDEPs. But it needs some other people to indicate that they like it too (or not) to move forward.

@jorisvandenbossche jorisvandenbossche marked this pull request as ready for review May 4, 2024 13:20
@jorisvandenbossche jorisvandenbossche changed the title Render PDEPs using sphinx Render PDEPs using myst-parser and the sphinx doc theme May 4, 2024
@rhshadrach
Copy link
Member

/preview

Copy link
Contributor

Website preview of this PR available at: https://pandas.pydata.org/preview/pandas-dev/pandas/51467/

@jorisvandenbossche
Copy link
Member Author

Example page to compare:

There are certainly a few things that could be further improved (e.g. the breadcrumb is quite useless, but that is configurable and could be left out), but personally I find that the version of this PR gives a better readable page

@rhshadrach
Copy link
Member

rhshadrach commented May 16, 2024

I share @datapythonista's concerns on having a page on the website (outside of the docs) that doesn't share the theme. At a quick glance, I think we can change how the PDEPs are currently built to handle at least some of the issues in the OP of #51461, and would like to take a stab at that to see where I get.

Currently, the color theme is not great in my browser, but I presume that is easy to fix.

image

@jorisvandenbossche
Copy link
Member Author

Currently, the color theme is not great in my browser, but I presume that is easy to fix.

Ah, yes (locally I don't default to dark theme, so didn't notice this). That can easily be forced to always be light (essentially defaulting to "light" and there is no button to change it on this page)

@jorisvandenbossche
Copy link
Member Author

concerns on having a page on the website (outside of the docs) that doesn't share the theme.

So then let's move the PDEPs out of the website, and then this argument is not relevant anymore? ;)

This is half a joke, but in practice, for example https://www.python.org/ has a link to the PEP index ("Documentation" dropdown -> "PEP index"), which then gives a page with a different styling. Similarly for numpy, the NEPs live in a suburl https://numpy.org/neps/ but built separately from the website and using different styling than https://numpy.org.

Personally, I think it is fine that the PDEPs are more part of the website while still using a different styling. Most people looking at the PDEP pages will also look a lot more at our docs than at our website, is my guess, so theming of the PDEPs using the doc style will actually feel more familiar

@rhshadrach
Copy link
Member

Personally, I think it is fine that the PDEPs are more part of the website while still using a different styling.

I don't think it's desirable (regardless of others that may have done it), but also not a big deal. For me, the larger point is the tech debt introduced - we now have three styles to support: sphinx in docs, web page, and sphinx in web page. Still, if people find the style is worth it, I'm not opposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Web pandas website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants