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

document class methods using sphinx3 recursive doc #1075

Merged
merged 8 commits into from
Oct 15, 2020

Conversation

veronicaguo
Copy link
Contributor

@veronicaguo veronicaguo commented Oct 6, 2020

@kandersolar
Copy link
Member

Thanks @veronicaguo for opening this PR! I just quickly clicked through some API Reference pages, so I don't have thoughts on #1055 but here are a couple points about bumping the sphinx version:

  • The list of references is improved over what I remember from sphinx 2.x, but still different from what we're seeing on 1.8.5. I'm okay with the new formatting and could be convinced it's even an improvement over the current format. Example: https://pvlib-python--1075.org.readthedocs.build/en/1075/generated/pvlib.iam.martin_ruiz.html
  • The in-text links are now bare numbers rather than numbers in brackets (e.g. 1 vs [1]). I personally prefer the bracketed version. If others feel the same, I think this custom CSS (see docs/sphinx/source/_static/) should bring back the brackets:
.footnote-reference::before {
  content: '[';
}
.footnote-reference::after {
  content: ']';
}

@veronicaguo
Copy link
Contributor Author

Thanks @kanderso-nrel for the feedback!

  • I agree that bracketed version for reference numbers looks better. will add the custom CSS.

Copy link
Member

@kandersolar kandersolar left a comment

Choose a reason for hiding this comment

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

Interestingly, the reference list on the clearsky page renders differently from the ones in the docstrings. I wonder if it's because it's using citations instead of footnotes?

https://pvlib-python--1075.org.readthedocs.build/en/1075/clearsky.html#references

Not related to this PR, just something I saw while clicking through the pages: the comparison with the MATLAB version could use a refresh, specifically the details about our continuous integration setup.

@wholmgren
Copy link
Member

Thanks @veronicaguo!

Interestingly, the reference list on the clearsky page renders differently from the ones in the docstrings. I wonder if it's because it's using citations instead of footnotes?

Also on the forecasting page: https://pvlib-python--1075.org.readthedocs.build/en/1075/forecasts.html#lar16

I have a mild preference for citation style for the narrative documentation.

@wholmgren wholmgren added this to the 0.8.1 milestone Oct 14, 2020
@wholmgren wholmgren changed the title 1055 recursive doc document class methods using sphinx3 recursive doc Oct 15, 2020
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

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

thanks @veronicaguo. @kanderso-nrel ok to merge?

Copy link
Member

@kandersolar kandersolar left a comment

Choose a reason for hiding this comment

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

LGTM!

@wholmgren wholmgren merged commit 3e25627 into pvlib:master Oct 15, 2020
@veronicaguo veronicaguo deleted the 1055-recursive-doc branch October 15, 2020 21:51
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.

class methods do not have doc pages
4 participants