Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Perform dayofyear-based calculations according to UTC, not local time #2055
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
Perform dayofyear-based calculations according to UTC, not local time #2055
Changes from all commits
6c4c32a
9ce02d5
0ad7a68
05961d2
45a36cb
b00a2d4
7bb7e02
6d1ab57
3fed282
6f98df7
fe5b5c1
97afcb8
defddfd
a08ca79
6baa1ed
4515366
83a61ba
745579c
b6d238a
fa62003
5d80876
3e95694
59cb82b
6e1c251
0baa338
cd45c74
1e6914d
dd78f3e
35e0756
758ea98
5102347
1c940fb
12c6a49
ab7d307
82aa0df
c4862b5
57d052e
85eae01
3fc5579
f255e8b
63df590
639ce7f
33f778f
16f0e5a
2745954
eb2707f
ab953b5
b8246c0
7da25f4
70d06cb
dea79d3
2143c94
80a6afb
87cd719
c4b65e1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 202 in pvlib/solarposition.py
pvlib/solarposition.py#L202
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree with this change. The docstring says the input timestamps "must be localized to the timezone for the longitude", which is inconsistent with the previous behavior (localize to UTC if not already localized), so I can see some change being needed here. But why is this the correct change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was added in accordance with
sun_rise_set_transit_spa
andsun_rise_set_transit_ephem
as well as the docstring. I agree with the previous convention of raising aValueError
since it prevents users from unwittingly calling the functions naively without assuming any timezones and ignoring the fact that an hour angle is counted up from zero at noon in a predefined local timezone. Please refer to the followings:pvlib-python/pvlib/solarposition.py
Lines 448 to 452 in 524fa55
pvlib-python/pvlib/solarposition.py
Lines 560 to 564 in 524fa55