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.
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
Implement Lorenz et al. POA and GHI QC methods #167
base: main
Are you sure you want to change the base?
Implement Lorenz et al. POA and GHI QC methods #167
Changes from 29 commits
4175a4f
0fd23e3
9399adf
60861b2
94d3430
792f10d
a2aa469
7097952
97f3a9c
8d2007b
a11d8d8
75dda23
6c56859
4da8913
7420547
e98d476
e0bfa2b
d0591cc
6d99699
e25674b
895bf72
98f8445
3ddfcc9
f99c530
ff5f019
8524090
6275efd
f94efc3
eef5429
c90a224
d144c68
165ce91
863ecde
57ae3bf
901e0a0
9e05e4c
771c721
acb8a40
368a6d0
3e27f93
8764c7b
ee9f400
74cac5a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 still in strong favor of not flagging values at nighttime. I recently had a chat with the main author of the paper who did not have a recommendation for night time. Lorenz pointed me toward one of the co-authors who has yet to respond to my request.
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.
Not having thought about the significance of nighttime data, I don't have a strong opinion on this. But if I were to think of irradiance as a part of operational analysis of a PV plant, I would like to have some distinction between daytime and nighttime data.
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 think this deserves it's own function.
The step change limit is fairly common:
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 could be done in a separate pull request.
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.
Good point. I think one good reason to have the step change limit in here is to stay true to the list of checks the paper suggests.
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.
we could have a helper function that returns the step threshold, and pass it a kwarg 'pvlive' to return 1000.
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.
As am I, let's do this and have a
step_limit
kwarg or something.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.
Can somebody help me with this? After some reading, I think I understand kwargs, but have never written a function with them. Some more guidance on how the functions' design would be helpful.
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.
@abhisheksparikh let's do that in a follow-on pull request. I feel like we are good enough here and we can improve later. Agree @AdamRJensen ?