Skip to content

WEB: Refine PDEP presentation #58791

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 5 commits into from
Jun 6, 2025

Conversation

rhshadrach
Copy link
Member

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.

@rhshadrach rhshadrach added Enhancement Web pandas website labels May 19, 2024
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;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

I think it's

h2:hover a.headerlink, h3:hover a.headerlink {
...
}

You can avoid repeating the content, but I don't think you can avoid repeating a.headerlink or other things in the label.

body = markdown.markdown(
content, extensions=context["main"]["markdown_extensions"]
)
if "pdeps/" in fname:
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

@rhshadrach
Copy link
Member Author

/preview

Copy link
Contributor

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

@rhshadrach
Copy link
Member Author

PDEP6 shows code highlighting.

cc @jorisvandenbossche @datapythonista

@jorisvandenbossche
Copy link
Member

@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:

@Aloqeely
Copy link
Member

Aloqeely commented May 20, 2024

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

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 Jun 20, 2024
@datapythonista
Copy link
Member

/preview

@datapythonista
Copy link
Member

@rhshadrach should we merge this?

@datapythonista
Copy link
Member

/preview

Copy link
Contributor

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

@rhshadrach
Copy link
Member Author

/preview

Copy link
Contributor

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

@rhshadrach
Copy link
Member Author

/preview

Copy link
Contributor

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

@datapythonista datapythonista merged commit eea6d0e into pandas-dev:main Jun 6, 2025
8 checks passed
@datapythonista
Copy link
Member

Thanks @rhshadrach, very nice. Maybe the ecosystem can also benefit from this, I'll create an issue.

@rhshadrach
Copy link
Member Author

This PR still suffered from the height issue that @jorisvandenbossche pointed out above.

@datapythonista
Copy link
Member

It works well in my browser, visualization is fine, and links work perfect. I use Brave on Linux. Not sure if I can't see the problem, or just works well for me.

Feel free to open an issue with the exact problem, if I can reproduce it happy to take a look, otherwise someone else will probably fix it if you label it as good first issue.

@rhshadrach
Copy link
Member Author

Weird I'm not seeing the same. Perhaps I was testing an old link when I last looked at this. But now I'm noticing that the Table of Contents is gone.

@rhshadrach
Copy link
Member Author

Ah, it's only on the Implemented pages.

@datapythonista
Copy link
Member

Only thing I was thinking is whether it's been this PR that decreased the width of the content of the website. Maybe just my perception, but I think it was wider (maybe it became narrower before).

But the tables of content look fine everywhere I checked, also in the implemented section, at list the first one: https://pandas.pydata.org/pdeps/0004-consistent-to-datetime-parsing.html

transition: opacity 0.5s;
}
.container {
max-width: 100ch;
Copy link
Member

Choose a reason for hiding this comment

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

Yep, this is what's making the page narrower. Seems intentional, do you think it looks better? I personally prefer the default bootstrap sizing.

Copy link
Member Author

Choose a reason for hiding this comment

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

The intention here was to decrease it only on PDEP pages. Looks like that's not what's happening. On pages that are more like articles, I do indeed prefer a more narrow text (this is pretty standard in academia).

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

Successfully merging this pull request may close these issues.

WEB: use sphinx to render the PDEPs
4 participants