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.
add functionality to load Solcast API data to
iotools
#1875New 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
add functionality to load Solcast API data to
iotools
#1875Changes from 3 commits
beba7ba
e731ee0
1f162cf
12c1fe3
30278c9
9dfc3d0
85f8f73
90983fe
499f51a
521e742
d79a764
7cb28fe
6792ecb
c0fd312
be27d85
a33b4b2
b41ab76
f2baf74
25448ce
a9dac8f
ec21bec
abdee5f
ba28943
f2b7323
c9788ad
52ab6d7
d45ddec
9be7792
6fc890b
dad2be7
aa5005d
593d8ac
26942dc
147a2b1
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.
@AdamRJensen This is an interesting approach that we might consider adopting in some centralized way for iotools in general.
No action needed in this PR, of course.
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.
This is pretty cool
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.
Does this only happen when
map_variables
is True?I think it would be preferable to always have the same time index convention, so I would move this comment to the output section.
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.
yes, it is applied only if
map_variables
is True. The idea being that the raw data is returned if set toFalse
, but we can do that by default if preferred?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.
Would it be an idea to expose the endpoint as a parameter? For example, in case you have different versions, e.g., tmy/radiation_and_weather/v2/?
This is of course easy to add later, but figured I'd raise the point now for discussion
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.
that is a valid point, and relevant for our SDK too. There are no use-cases for this functionality yet, but I agree that down the line we may need expose this to the user.
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 unfamiliar with the term "actuals" but maybe that is standard? As a non-native, I can't really tell what meaning is trying to be conveyed
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 refer to "live" and "historical" data as estimated actuals. These would be sometimes be called "measurements" I guess, but we avoid that term as we are really just estimating these values from satellite observations and not direct measurements of weather/power. Safe to assume that anyone using the Solcast API understands this?
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.
Hmm I think that's a very Solcast specific terminology. To be more in line with the other iotools I suggest removing mentions of "actuals" and just calling them "estimates"