Skip to content

Expand docstring for aoi_projection. #648

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 1 commit into from
Jan 21, 2019
Merged

Expand docstring for aoi_projection. #648

merged 1 commit into from
Jan 21, 2019

Conversation

adriesse
Copy link
Member

@adriesse adriesse commented Jan 19, 2019

  • Enhances the solution for limit cos(aoi) in diffuse sky functions #535
  • I am familiar with the contributing guidelines.
  • Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.
  • Code quality and style is sufficient. Passes LGTM and SticklerCI checks.
  • New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
  • Pull request is nearly complete and ready for detailed review.

Calculates the dot product of the solar vector and the surface
normal.
Calculates the dot product of the sun position unit vector and the surface
normal unit vector; in other words, the cosine of the angle of incidence.
Copy link
Member

Choose a reason for hiding this comment

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

to improve this even slightly more, can you use LaTeX to add the equation?

$$AOI_{projection} = \\cos{ \\beta } \\cos{ \\left( \\text{ solar zenith } \\right) } + \\sin{ \\beta } \\sin{ \\left( \\text{ solar zenith } \\right) } \\cos{ \\left( \\text{ solar azimuth} \\right) - \\left( \\text{ surface azimuth} \\right) }$$

might require some creative thinking about symbols, and some text here or elsewhere to define them. Just an idea

Copy link
Member Author

Choose a reason for hiding this comment

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

Next time?

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 @adriesse.

Ok with me to merge as is.

@cwhanse
Copy link
Member

cwhanse commented Jan 21, 2019

thanks @adriesse

@cwhanse cwhanse closed this Jan 21, 2019
@cwhanse cwhanse reopened this Jan 21, 2019
@cwhanse cwhanse merged commit 52eeae7 into pvlib:master Jan 21, 2019
@wholmgren wholmgren added this to the 0.6.1 milestone Jan 21, 2019
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.

4 participants