Add NSRDB PSM4 polar and polar TMY iotools#2807
Conversation
AdamRJensen
added
remote-data
AdamRJensen
added
remote-data
…sen/pvlib-python into add-nsrdb-polar-iotools
AdamRJensen
added
remote-data
AdamRJensen
added
remote-data
AdamRJensen
added
remote-data
|
Ready for review! Test failures are unrelated. |
There was a problem hiding this comment.
I think the only (?) difference between the get_nsrdb_psm4_polar and get_nsrdb_psm4_polar_tmy is that the second has year='tmy' (and a different URL). I assume you split it into two functions to follow the iotools convention, but honestly I could easily see it being one function...
I guess you could add a test for a bad request (api_key or location).
Otherwise LGTM (but I couldn't evaluate the readthedocs)!
| latitude : float or int | ||
| in decimal degrees, between 60 and 90 degrees north | ||
| longitude : float or int | ||
| in decimal degrees, between -180 and 180, east is positive |
There was a problem hiding this comment.
I couldn't easily find the locations where data exists. Is there some link we could provide or something (even the NSRDB API example is a bit confusing because they report longitude first...)
There was a problem hiding this comment.
This info has been added to the documentation:
The dataset provides data from 2013 onwards over the exclusive economic zone above 60 degrees north.
There was a problem hiding this comment.
No ok, I guess my api key did not work yesterday, and I thought that you couldn't ask for any set of coordinates. I tried it now and it worked fine. All good!
There was a problem hiding this comment.
So it's available above 60N for all longitudes? If so, then I'd remove the added sentence about EEZ, that's just awkward.
AdamRJensen
added
remote-data
Tests have been added for erroneous inputs (bad location and bad API key). |
AdamRJensen
added
remote-data
The separate functions for PSM4 vs. PSM4_TMY was decided here: #2326 (comment) |
| """ | ||
| Retrieve timeseries weather data from the PSM4 NSRDB Polar API. | ||
|
|
||
| The NSRDB is described in [1]_ and the NSRDB PSM4 NSRDB Polar v4 dataset |
There was a problem hiding this comment.
| The NSRDB is described in [1]_ and the NSRDB PSM4 NSRDB Polar v4 dataset | |
| The NSRDB is described in [1]_ and the NSRDB Polar dataset |
| The dataset provides data from 2013 onwards over the exclusive economic | ||
| zone above 60 degrees north. The data have a spatial resolution of 4 km |
There was a problem hiding this comment.
EEZ is an awkward description, but taken from the NSRDB webpage. I've contacted them to see if they can clarify.
There was a problem hiding this comment.
I agree, but looking at maps from the NSRDB Polar, then the data availability matches the EEZ.
I've updated the latitude/longitude parameters to mention this.
There was a problem hiding this comment.
Ok, if the data don't extend to the pole. then we need something like that statement.
But there are multiple EEZs (they are national, not one international). What about "...above 60 degrees north and within an exclusive economic zone."
| latitude : float or int | ||
| in decimal degrees, between 60 and 90 degrees north | ||
| longitude : float or int | ||
| in decimal degrees, between -180 and 180, east is positive |
There was a problem hiding this comment.
So it's available above 60N for all longitudes? If so, then I'd remove the added sentence about EEZ, that's just awkward.
| The NSRDB is described in [1]_ and the NSRDB PSM4 Polar TMY v4 dataset is | ||
| described in [2]_, [3]_. The polar dataset extends NSRDB coverage to high | ||
| latitude polar regions, that are outside the field of view of the | ||
| geostationary satellites. | ||
|
|
||
| The dataset provides typical year data over the exclusive economic zone | ||
| above 60 degrees north. The data have a spatial resolution of 4 km and a | ||
| temporal resolution of 1 hour. |
There was a problem hiding this comment.
Mirror the text from get_nsrdb_psm4_polar
There was a problem hiding this comment.
The text is already almost identical for the two functions, with the only difference being that the get_nsrdb_psm4_polar_tmy function mentions "typical meterological year".
There was a problem hiding this comment.
I only meant a reminder to update this text, when comments about EEZ are resolved for the other function.
|
Need to add two lines to this file: https://github.com/pvlib/pvlib-python/blob/main/docs/sphinx/source/reference/iotools.rst |
@cwhanse I don't understand this comment, the two functions have already been added to the page. Maybe I'm missing something. |
I missed it. Please disregard. |
3 participants
docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor 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`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.