Skip to content

Bump docs infraestructure #2112

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 39 commits into from
Jul 14, 2024
Merged

Bump docs infraestructure #2112

merged 39 commits into from
Jul 14, 2024

Conversation

echedey-ls
Copy link
Contributor

@echedey-ls echedey-ls commented Jun 29, 2024

  • [ ] Closes #xxxx
  • [ ] I am familiar with the contributing guidelines
  • [ ] Tests added
  • [ ] Updates entries in docs/sphinx/source/reference for API changes.
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
  • [ ] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

I hate going so much behind the latest versions. Here is the docs updated.

Issues

Link

https://pvlib-python--2112.org.readthedocs.build/en/2112/

@echedey-ls echedey-ls marked this pull request as ready for review June 29, 2024 13:18
@RDaxini
Copy link
Contributor

RDaxini commented Jul 1, 2024

do people actually like the "previous/next" links?

Personally, I've never used them... I also use the sidebar. Chances of someone needing to browse through the functions in the order of those links seems small.


Not sure if it is an issue or just the way the new docs are, but I miss the colour of block code. I feel like the more distinct colours in the old docs (including the In / Out) improves the readability of the code, at least for me anyway.
New:
new
Old:
old

Surprisingly, I find the slightly smaller text easier to read too. Not sure why... a bit less in-your-face maybe?

@RDaxini
Copy link
Contributor

RDaxini commented Jul 2, 2024

The RMS value on the final figure for the Fast simulation using the ADR efficiency model starting from PVsyst parameters gallery example in the new docs is different to the RMS value in the old docs (0.16 vs 0.15)
Is this an issue?

@cwhanse
Copy link
Member

cwhanse commented Jul 2, 2024

The RMS value on the final figure for the Fast simulation using the ADR efficiency model starting from PVsyst parameters gallery example in the new docs is different to the RMS value in the old docs (0.16 vs 0.15) Is this an issue?

@adriesse

@AdamRJensen
Copy link
Member

AdamRJensen commented Jul 5, 2024

I've clicked around a bunch and most things are looking good to me. Thanks for taking this initiative @echedey-ls!

When we're ready, I suggest that we wait and merge this right after the next release. Then all reviews of new PRs will be using the new documentation layout, which gives us time until the next release to passively detect any potential issues.

Note, I've added an issue in the PR description concerning "Pages without any section still feature the "Section Navigation"". Also, it seems to me that the copyright isn't rendered anywhere.

@AdamRJensen
Copy link
Member

AdamRJensen commented Jul 5, 2024

Let's do a more rigorous walk-through where we make sure that all pages have been skimmed. Here's a proposal for how to delegate the documentation:

Co-Authored-By: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
@IoannisSifnaios
Copy link
Contributor

IoannisSifnaios commented Jul 5, 2024

So I skimmed through the entire Example Gallery and found the following issues:

  • All the View on GitHub links are broken (this was recently fixed, but maybe the new sphinx uses different paths?)
  • For some reason, the references in examples 1, 2, 3, 4, 5 render using the old format (where the number of times each citation is used shows up next to them):
image

However, there were many cases in which the references were rendered differently. Not sure why this happens...

  • Is it possible to remove these useless Out: boxes (unless we actively request something to be printed):
image

Otherwise, things look fine! :-)

@echedey-ls
Copy link
Contributor Author

echedey-ls commented Jul 5, 2024

  • All the View on GitHub links are broken (this was recently fixed, but maybe the new sphinx uses different paths?)

That was me not updating the branch from main 😬

  • For some reason, the references in examples [...]
    However, there were many cases in which the references were rendered differently. Not sure why this happens...

What is differently? [1] just with the link, no parenthesis with the occurrences like [1](1,2)? I think that's the default; I'm -1 to CSS-remove it since that would give us some references with the occurrence link and some others without it.

  • Is it possible to remove these useless Out: boxes (unless we actively request something to be printed):
image

You may find this link informative: https://sphinx-gallery.github.io/stable/auto_examples/plot_3_capture_repr.html#matplotlib-output
It seems a sphinx-gallery feature more than a customization left to the end users.

Otherwise, things look fine! :-)

Thx :) ❤️

Co-Authored-By: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
@adriesse
Copy link
Member

adriesse commented Jul 5, 2024

The RMS value on the final figure for the Fast simulation using the ADR efficiency model starting from PVsyst parameters gallery example in the new docs is different to the RMS value in the old docs (0.16 vs 0.15) Is this an issue?

@adriesse

Always curious why things change, but I do not see it as a problem.

Co-Authored-By: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
@IoannisSifnaios
Copy link
Contributor

IoannisSifnaios commented Jul 6, 2024

What is differently? [1] just with the link, no parenthesis with the occurrences like [1](1,2)? I think that's the default; I'm -1 to CSS-remove it since that would give us some references with the occurrence link and some others without it.

You are correct, my bad. It is just that in some cases, the References looked like the one below (which I prefer personally), and I thought they had changed something, but I guess the reason was that it was mentioned only once in the text...
image

You may find this link informative: https://sphinx-gallery.github.io/stable/auto_examples/plot_3_capture_repr.html#matplotlib-output
It seems a sphinx-gallery feature more than a customization left to the end users.

Thank you for the link! Yes, ok, I guess a fix could be to have the plt.show() at the end; but this could be for another PR!

@RDaxini
Copy link
Contributor

RDaxini commented Jul 7, 2024

I read through all sections of the user guide and found one issue and one difference.
One issue is that the bullet points and sub-bullet points have disappeared from the User Guide main page in the new docs.
One difference is the layout of the footer. It is not a major issue, but from a stylistic perspective my preference leans towards the old layout.
I will have another scan through at another time just in case I missed anything but this is all I have found for now.
Images for both pointers below:
Here's the new main page:
new main
Here's the old main page:
old main
Here's the new footer layout:
new layout
Here's the old footer layout:
old layout

@AdamRJensen
Copy link
Member

I read through all sections of the user guide and found one issue and one difference. One issue is that the bullet points and...

@RDaxini You have keen eyes! However, I don't think that's per se an issue but simply a style change. To reduce the maintenance and because the changes are minor, I suggest we simply just accept these small changes.

@echedey-ls in #1359 we made some changes to the position of the placement of the ethical ads (I don't think this fix is necessary any longer). This means that we can leave html_sidebars unspecified which defaults to "**": ["sidebar-nav-bs", "sidebar-ethical-ads"] and the footer_end can be set to empty {} if we don't want to show the theme version. footer_start and footer_center should also then be unspecified, i.e., left to their default values.

@kandersolar
Copy link
Member

I checked my sections (#2112 (comment)) and found nothing that should be fixed in this PR!

@kandersolar kandersolar added this to the v0.11.1 milestone Jul 10, 2024
echedey-ls and others added 2 commits July 13, 2024 11:14
Co-Authored-By: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
@echedey-ls
Copy link
Contributor Author

@AdamRJensen thx for the input on the footer and navbar.

Out of the 140 functions in my sections, I have compared roughly a third, taking special care to features like tables, figures and constants documentation. Everything looks good to me. Maybe the pvsystem class table that lists it is much more prominent in the new docs, thou I don't find it as an issue since it also comes from a well-formed toc tree.

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
@echedey-ls
Copy link
Contributor Author

Looks like #2112 (comment) list is already full. Do you all miss anything else? I think we are good to go, feedback is welcome.

@kandersolar
Copy link
Member

Everyone has checked their sections, @echedey-ls is happy, let's merge it!

Thanks @echedey-ls for the initiative, and the broader group for the collective review!

@kandersolar kandersolar merged commit 8e2bc91 into pvlib:main Jul 14, 2024
30 checks passed
@echedey-ls echedey-ls mentioned this pull request Jul 18, 2024
3 tasks
@echedey-ls echedey-ls deleted the bump-docs branch October 15, 2024 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants