Skip to content

Add pvlib.spa.transit_sunrise_sunset to documentation - #2873

Merged
kandersolar merged 9 commits into
pvlib:mainfrom
AdamRJensen:add_transit_sunrise_sunset_to_docs
Oct 28, 2026
Merged

kandersolar merged 9 commits into
pvlib:mainfrom
AdamRJensen:add_transit_sunrise_sunset_to_docs

Conversation

@AdamRJensen

Copy link
Copy Markdown
Member
  • Closes transit_sunrise_sunset: seems as a public function, not in public docs? #2255
  • I am familiar with the contributing guidelines
  • I attest that all AI-generated material has been vetted for accuracy and is in compliance with the pvlib license
  • Updates entries in docs/sphinx/source/reference 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 (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Add transit_sunrise_sunset function to documentation. See #2255.

@AdamRJensen AdamRJensen added this to the v0.16.2 milestone Sep 29, 2026
Comment thread pvlib/spa.py Outdated
Comment thread pvlib/spa.py Outdated
Comment thread pvlib/spa.py Outdated
Comment thread pvlib/spa.py
@kandersolar

Copy link
Copy Markdown
Member

I would like to know why github is now showing the entire SPA file as one big change.

@echedey-ls

Copy link
Copy Markdown
Member

I would like to know why github is now showing the entire SPA file as one big change.

~/Documents/10_Programming/pvlib-python remotes/AdamRJensen/add_transit_sunrise_sunset_to_docs* ≡
❯ file pvlib/spa.py
pvlib/spa.py: Python script, Unicode text, UTF-8 text executable, with CRLF line terminators

You probably need to mitigate with https://docs.github.com/en/get-started/git-basics/configuring-git-to-handle-line-endings?platform=windows
The world would be a better place without windows.

@kandersolar

Copy link
Copy Markdown
Member

I thought maybe it was line ending mismatch, but I'm confused:

  1. I had hoped we were free from line ending issues after [Maint] Line ending normalization wrong in some files #2802.
  2. The issue showed up here after I pushed commits using github.com (not from a local windows repo).

So I do not see how a mismatch would appear now...

@echedey-ls

Copy link
Copy Markdown
Member

Hmmm good point. I definitely don't have a clue now that you point that out.

The commit where some (not all) line end with \r (nonprintable return, CR of 'CRLF') is the first "Apply batched suggestions from code-review" co-authored by Dax (969a770).

~/Documents/10_Programming/pvlib-python add_transit_sunrise_sunset_to_docs~2* ≡
venv ❯ rg '\r' pvlib/spa.py
1111:    for a set of dates at a given location using the NREL SPA [1]_, [2]_.
1137:    References
1138:    ----------
1139:    .. [1] I. Reda and A. Andreas, Solar position algorithm for solar
1140:       radiation applications. Solar Energy, vol. 76, no. 5, pp. 577-589,
1141:       2004. :doi:`10.1016/j.solener.2003.12.003`
1142:    .. [2] I. Reda, and A. Andreas. "Corrigendum to “Solar position 
1143:       algorithm for solar radiation applications." [Solar Energy 76
1144:       (2004) 577–589] Solar Energy, vol. 81,  no. 6, pp. 838, 2007.
1145:       :doi:`10.1016/j.solener.2007.01.003`
1146:   """

In fact, this are exactly the same lines suggested by him in this review comment: #2873 (comment)

And given the content, I guess that's copy&pasted from somewhere else. I guess GH gives a shit regarding our git configuration? Super weird, but not so much considering Microslop bought GH years ago.

@RDaxini

RDaxini commented Oct 27, 2026

Copy link
Copy Markdown
Member

The commit where some (not all) line end with \r (nonprintable return, CR of 'CRLF') is the first "Apply batched suggestions from code-review" co-authored by Dax (969a770).

~/Documents/10_Programming/pvlib-python add_transit_sunrise_sunset_to_docs~2* ≡
venv ❯ rg '\r' pvlib/spa.py
1111:    for a set of dates at a given location using the NREL SPA [1]_, [2]_.
1137:    References
1138:    ----------
1139:    .. [1] I. Reda and A. Andreas, Solar position algorithm for solar
1140:       radiation applications. Solar Energy, vol. 76, no. 5, pp. 577-589,
1141:       2004. :doi:`10.1016/j.solener.2003.12.003`
1142:    .. [2] I. Reda, and A. Andreas. "Corrigendum to “Solar position 
1143:       algorithm for solar radiation applications." [Solar Energy 76
1144:       (2004) 577–589] Solar Energy, vol. 81,  no. 6, pp. 838, 2007.
1145:       :doi:`10.1016/j.solener.2007.01.003`
1146:   """

In fact, this are exactly the same lines suggested by him in this review comment: #2873 (comment)

And given the content, I guess that's copy&pasted from somewhere else. I guess GH gives a shit regarding our git configuration? Super weird, but not so much considering Microslop bought GH years ago.

I don't really understand what happened here either... did I do something wrong in the way I left a comment? 😂
What does "line end with \r (nonprintable return, CR of 'CRLF')" mean? See attached image, that's how my comments looks to me when I typed it (no \r's anywhere...)
What do you mean given the content it's likely copy/pasted?
When making suggestions on a PR, I do sometimes use online tools (e.g. textmechanic.com) to ensure I don't go over 79 characters and break the flake8 test for the contributor. That involves me pasting my commented suggestion into the tool and then copying the wrapped output back out. No recollection of whether I did that specifically in this PR though, but I would not have thought that'd be an issue(?)

image

@echedey-ls

Copy link
Copy Markdown
Member

@RDaxini you are not to blame, this platform is (if you ever are, I'll make sure to ping you and be explicit).

In fact, now that I think more about it, even if your procedure is making this unwanted change, I believe in the past that was not an issue. I'm unlikely to confirm this issue since I don't have Windows anymore and probably can't get close enough to reproduce.

FYI, CRLF/LF are the "new line" or "end of line" substring in text files, in Windows and Unix respectively. Windows uses two ASCII control characters (two bytes), CRLF, aka carriage-return and line-feed, aka represented by "\r\n" in programming languages such as C, Python and the regex I provided to ripgrep (rg) the command above in my comment. They are "control characters", non-printable by definition, so you won't see them rendered unless using specific text editors. Unix does the same to represent end of line, just by using one byte, LF aka '\n'. I think you can find more info on the web regarding ASCII, escape sequences (that backslash something representation in Python and other languages), CRLF/LF normalization and ripgrep if you don't know what it is. Feel free to ask me, thou.

With copy&pasting I don't know if carriage returns can even be pasted, it's all pure speculation. Depends on so many things, OS, web browser, and GitHub API in this case.

@kandersolar kandersolar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still not really sure what happened here, but it's fixed now :)

@kandersolar
kandersolar merged commit c7c65f4 into pvlib:main Oct 28, 2026
27 checks passed
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.

transit_sunrise_sunset: seems as a public function, not in public docs?

5 participants