Skip to content

Conversation

echedey-ls
Copy link
Contributor

@echedey-ls echedey-ls commented Nov 25, 2022

Does not apply:

  • Tests added
  • Updates entries in docs/sphinx/source/reference for API changes.
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.

Description of issue in #1561
First time contributor here! Looking forward to make bigger PRs in short, so all tips and criticism is welcomed.

@echedey-ls
Copy link
Contributor Author

I've got a doubt in the optional use of ~ in :py:func:`~pvlib.iam.martin_ruiz`, I've seen some rST entries don't use it (I didn't) but all still works. Can you tell me if it's use is recommended or not?

@echedey-ls echedey-ls marked this pull request as ready for review November 25, 2022 11:31
@kandersolar kandersolar added this to the 0.9.4 milestone Nov 25, 2022
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.

Looks great, thanks @echedey-ls!

I've got a doubt in the optional use of ~ in :py:func:~pvlib.iam.martin_ruiz, I've seen some rST entries don't use it (I didn't) but all still works. Can you tell me if it's use is recommended or not?

The ~ controls whether the HTML link text will look like pvlib.iam.martin_ruiz or just martin_ruiz. See https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#cross-referencing-syntax

Sometimes it makes sense to use ~ and sometimes it's better not to use it. For this PR I would leave it the way it is now, but it's really just down to personal taste.

@echedey-ls
Copy link
Contributor Author

Thanks for the reference @kanderso-nrel!

@kandersolar
Copy link
Member

Looking forward to make bigger PRs in short

Please let us know if we can help :) and thanks again for this PR!

@kandersolar kandersolar merged commit 582b956 into pvlib:main Nov 28, 2022
@echedey-ls echedey-ls deleted the iam.martin_ruiz-docstring-fix branch September 12, 2023 13:43
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.

Equation in docstring of pvlib.iam.martin_ruiz does not match code
4 participants