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.

Sign up for GitHub

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

Jump to bottom

WEB: Refine PDEP presentation #58791

Open

rhshadrach wants to merge 1 commit into pandas-dev:main from rhshadrach:doc_pdep_publishing

+58 −33

Member

rhshadrach commented May 19, 2024

closes WEB: use sphinx to render the PDEPs #51461 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Alternative to #51467. I believe this address all items in the linked issue. The main benefit in not switching to publishing PDEPs in sphinx is that it then matches the style of the rest of the webpage.

This is just a rough version to demonstrate what's possible, some refinements (highlighted below) need to be made if we are to merge this.


          WEB: Refine PDEP presentation

9b66357

rhshadrach added Enhancement Web labels

rhshadrach requested a review from datapythonista as a code owner

May 19, 2024 20:07

rhshadrach commented

View reviewed changes

web/pandas/static/css/pandas.css

Comment on lines +115 to +122

+              h2:hover a.headerlink {
+                  opacity: 1;
+                  transition: opacity 0.5s;
+              }
+              h3:hover a.headerlink {
+                  opacity: 1;
+                  transition: opacity 0.5s;
+              }

Member Author

rhshadrach May 19, 2024

I think we should be able to combine these two, but wasn't able to figure out the syntax.

web/pandas_web.py

-                              body = markdown.markdown(
-                                  content, extensions=context["main"]["markdown_extensions"]
-                              )
+                              if "pdeps/" in fname:

Member Author

rhshadrach May 19, 2024

Because of the use of TocExtension below, we can't use the usual yaml. Not sure if there is a work around or a better approach here.

Member Author

rhshadrach commented May 19, 2024

/preview

Contributor

github-actions bot commented May 19, 2024

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

Member Author

rhshadrach commented May 19, 2024

PDEP6 shows code highlighting.

cc @jorisvandenbossche @datapythonista

Member

jorisvandenbossche commented May 20, 2024

@rhshadrach thanks a lot for looking into this!

I certainly think this is a nice improvement, and if we don't have agreement on using sphinx instead, we should definitely merge this one. But if you ask my honest opinion, I personally still think #51467 gives the better result.

Concrete feedback on the changes here:

The header links need some offset (it doesn't know about the top navbar). Eg https://pandas.pydata.org/preview/pandas-dev/pandas/58791/pdeps/0006-ban-upcasting.html#detailed-description

I think with the sphinx theme this is solved with
```
html {
  scroll-padding-top: calc(var(--pst-header-height) + 1rem);
}
```
(and here this can be be fixed number, I assume)
I would personally further reduce the width

Member

Aloqeely commented May 20, 2024 •

edited

Personal preference, but I think the "On this page" sidebar (from @jorisvandenbossche's PR) is cleaner and resembles the docs more than the table of contents here. (I guess that's a benefit of using Sphinx?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment