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

docs(python): Improve API reference landing page #14888

Merged
merged 4 commits into from
Mar 13, 2024

Conversation

alexander-beedie
Copy link
Collaborator

@alexander-beedie alexander-beedie commented Mar 6, 2024

Meaningful improvements to the presentation of the Python API docs, focusing on the primary landing page. "A picture tells a thousand words", so screenshots attached ;)

(There's a lot more we can do here, but this already feels like a decent upgrade).

Before

  • A strangely ordered list, meandering down the page; little structure or organisation. Not clear where each section begins/ends, as everything runs into everything else.

  • Ugly vertical scrollbar appearing in the left navbar.

    Python API landing page (before)

After

  • Grid/card layout featuring the same elements, but the most important classes now appear in something resembling priority order (DataFrame → LazyFrame → Series → Expressions→ ...)

  • Minor custom CSS directive takes care of the ugly vertical scrollbar.

  • Update: have also now sorted the IO section for better readability.

    Python API landing page (after)

Also

  • The somewhat useless docs root page now automatically redirects to this (the page above), which is the "real" primary landing page. No more linking through to what looks like a broken/incomplete page.

@github-actions github-actions bot added documentation Improvements or additions to documentation python Related to Python Polars rust Related to Rust Polars labels Mar 6, 2024
@alexander-beedie alexander-beedie changed the title docs: Improve Python API docs docs: Improve Python API docs landing page Mar 6, 2024
@MarcoGorelli
Copy link
Collaborator

css is alien to me but the result looks really nice, well done

@mcrumiller
Copy link
Contributor

mcrumiller commented Mar 6, 2024

I like this in general, but I have a few suggestions (obviously my opinion, feel free to disagree!):

  • The boxes are not well-defined: how are items within a box grouped? Can we get small headers for each box?
  • To really clean it up (maybe too much?) can we make each box "expandable"? i.e. it only includes the header, and has a down-arrow (like the left-panel list). That way you only see e.g. Input/Output, but when you click the down-arrow, the box expands and you see all of the options. As it is, I think it's a little hard to "search through" and find what you need. This would also give room to add a short description to each box, which I think would be extremely helpful.
  • Can we remove the underlines in the links in the boxes?
  • We should probably say "in the polars.* namespace. Also, what exactly does "public" mean for a Python API? Do we need this sentence? I feel it was added as filler because it felt empty otherwise.
  • On the bottom left, the Previous/Index link should be removed. It exists in the current API page and it brings you a completely empty page. I assume this is autogenerated, so perhaps we can make this page be html/index.html instead of the reference/index.html.

@MarcoGorelli
Copy link
Collaborator

Also, what exactly does "public" mean for a Python API?

check https://docs.pola.rs/development/versioning/ (anything non-public wouldn't go through a deprecation cycle)

@stinodego
Copy link
Contributor

I like the organization in boxes. The ordering in the menu is debatable but the new order is fair enough (I would keep DataFrame/Series together followed by LazyFrame/Expressions).

I need a moment to consider the redirect, not 100% sure it's a good idea.

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.01%. Comparing base (ac0131a) to head (cac54df).
Report is 55 commits behind head on main.

❗ Current head cac54df differs from pull request most recent head 09a0a71. Consider uploading reports for the commit 09a0a71 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14888   +/-   ##
=======================================
  Coverage   81.00%   81.01%           
=======================================
  Files        1331     1331           
  Lines      172728   172774   +46     
  Branches     2455     2455           
=======================================
+ Hits       139922   139973   +51     
+ Misses      32339    32334    -5     
  Partials      467      467           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexander-beedie
Copy link
Collaborator Author

alexander-beedie commented Mar 6, 2024

I need a moment to consider the redirect, not 100% sure it's a good idea.

It doesn't affect the docs indexing (the redirect is injected after the pages are generated from the toc refs). I had thought it could become some sort of "Welcome to the docs" splash page, but splash pages rapidly become something you want to skip past once you've seen them more than once or twice, so... I suggest we skip it for now so that the reader will always go directly to the more useful landing page; if we come up with something worthwhile later then we can simply remove the redirect - it doesn't commit us to anything, it just tidies things up for now.

@alexander-beedie
Copy link
Collaborator Author

alexander-beedie commented Mar 6, 2024

Can we get small headers for each box?

Yup; not hard at all, but can be done in a second pass if somebody comes up with good titles. This is a meant as a decent first step that can be built/iterated on.

To really clean it up (maybe too much?) can we make each box "expandable"?

Definitely too much; this isn't about hiding things so much as better-organising useful information.

We should probably say "in the polars.* namespace.

Done.

On the bottom left, the Previous/Index link should be removed // ... // I assume this is autogenerated.

It is; if the links aren't going somewhere useful and you can work out how to tweak/redirect, let me know ;)

@mcrumiller
Copy link
Contributor

mcrumiller commented Mar 6, 2024

It is; if the links aren't going somewhere useful and you can work out how to tweak/redirect, let me know ;)

Can't you transfer the content of reference.html to index.html and remove reference.html entirely?

Edit: I see, this is generated by Sphynx.

@alexander-beedie alexander-beedie force-pushed the python-api-docs branch 2 times, most recently from f04feef to 1343d31 Compare March 6, 2024 19:35
@ritchie46
Copy link
Member

Can we merge this? Looks nice. :)

@alexander-beedie
Copy link
Collaborator Author

Can we merge this? Looks nice. :)

@stinodego - final thoughts on the redirect? Doesn't have to be permanent if you can think of something better to do with that root page, but currently it just looks broken 😅

@stinodego
Copy link
Contributor

stinodego commented Mar 13, 2024

I wouldn't say it looks broken, but it definitely isn't really useful as we use Sphinx only for the API reference.

The redirect is fine, but we have to make sure we point directly to the reference/index.html everywhere else. I fixed a link in this repo, and will possibly have to update some stuff elsewhere (our website mostly). We wanted to change the URL path anyway so I'll pick that up in one go.

@stinodego stinodego changed the title docs: Improve Python API docs landing page docs(python): Improve Python API docs landing page Mar 13, 2024
@stinodego stinodego removed the rust Related to Rust Polars label Mar 13, 2024
@stinodego stinodego changed the title docs(python): Improve Python API docs landing page docs(python): Improve API reference landing page Mar 13, 2024
@stinodego stinodego merged commit 1d7b49e into pola-rs:main Mar 13, 2024
17 checks passed
@alexander-beedie alexander-beedie deleted the python-api-docs branch March 13, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants