Skip to content

[STY] remove old comments from single axis tracking #1041

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

Closed
mikofski opened this issue Sep 3, 2020 · 0 comments
Closed

[STY] remove old comments from single axis tracking #1041

mikofski opened this issue Sep 3, 2020 · 0 comments

Comments

@mikofski
Copy link
Member

mikofski commented Sep 3, 2020

Describe the bug
After #823 is merged there may be stale comments in pvlib.tracking.singleaxis and commented code that can be removed. This might make the code more readable. It would also resolve some stickler complaints about long lines.

To Reproduce
Comments to remove:

  1. L375-L379 - the tracking algorithm now follows [1] that uses clockwise rotation around z-axis from north
  2. L393-L395 - ditto
  3. L400-L410 - ditto
  4. L441-L452 - pvlib has been using arctan2(x,z) in pvlib.tracking.singleaxis for 6 years since 1fb82cc, so I believe these comments are unnecessary now
  5. L471-L472 - this commented code was updated in Singleaxis tracking with non-parallel slope #823, should we leave it or delete it?
  6. L553-L555

etc.

[1] https://www.nrel.gov/docs/fy20osti/76626.pdf

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Versions:

  • pvlib.__version__:
  • pandas.__version__:
  • python:

Additional context
Add any other context about the problem here.

mikofski added a commit to mikofski/pvlib-python that referenced this issue Sep 3, 2020
* remove commented code, especially any code relating to use of arctan2
  adopted in 1fb82cc
* remove comments that should be sourced directly from refernces, and
  instead list reference and Eqs. numbers
* tidy up, combine lines, etc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant