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

[DOC] add TEMPERATURE_MODEL_PARAMETERS to api.rst #1036

Merged
merged 4 commits into from
Sep 2, 2020

Conversation

mikofski
Copy link
Member

@mikofski mikofski commented Aug 27, 2020

  • Closes TEMPERATURE_MODEL_PARAMETERS is undocumented #939
  • I am familiar with the contributing guidelines
  • Tests added no code, only documentation
  • Updates entries to docs/sphinx/source/api.rst 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 and Milestone are assigned to the Pull Request and linked Issue.

add TEMPERATURE_MODEL_PARAMETERS to the documentation

@mikofski
Copy link
Member Author

See if you like what this looks like in the API reference
image
and where it's referenced in the docs, EG: pvlib.temperature.sapm_cell
image
and in the module index
image

@mikofski
Copy link
Member Author

also if it's okay to make an exception for stickler, it's a long line in the example of the docstring, I don't think it will look as good wrapped, but maybe?

@wholmgren
Copy link
Member

Thanks @mikofski! Is it easy to put the whole docstring on its own page and keep only the first line on the rendered api.rst? I'm thinking of something like the table entries that surround it. Not a big deal for a single constant but if we add more then a lot of the real estate on the rendered page will be used for constants.

pvlib/temperature.py Outdated Show resolved Hide resolved
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
@mikofski
Copy link
Member Author

I had troubles with that, b/c the autosummary directive only works on classes and functions that have the __module__ attr - so it adds TEMPERATURE_MODEL_PARAMS to the table as a one-liner, but then the link goes to a 404 page not found default page. There may be an option to manually add something to api.rst, that's not part of the table, and link it to a new page in the index.rst but then it won't be part of the api tree in the left margin What do you think of a page just for constants?

@wholmgren
Copy link
Member

wholmgren commented Aug 27, 2020

I was afraid it wasn't going to be easy but thanks for checking. I suggest we keep them on api.rst until it becomes a problem.

@wholmgren wholmgren added this to the 0.8.0 milestone Sep 1, 2020
pvlib/temperature.py Outdated Show resolved Hide resolved
@wholmgren wholmgren mentioned this pull request Sep 2, 2020
8 tasks
@mikofski
Copy link
Member Author

mikofski commented Sep 2, 2020

Thanks @wholmgren I've fixed the stickler issues, and caught a typo. I think it's good now

@wholmgren
Copy link
Member

Thanks @mikofski! Test failure is unrelated so merging.

@wholmgren wholmgren merged commit 4c8a49f into pvlib:master Sep 2, 2020
@mikofski mikofski deleted the doc-temp-model-params branch September 6, 2020 22:24
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.

TEMPERATURE_MODEL_PARAMETERS is undocumented
3 participants