ENH: Create psm3.py #694
ENH: Create psm3.py #694
Conversation
initial concept for psm3 reader
|
@mikofski how does this pass lat / long ? |
|
@cwhanse the arguments for |
|
Aha I see it now. |
|
There are a lot of typos in here, but it does work, I'll fix the typos and upload a test ASAP |
* need to strip all leading whitespace, and only one whitespace between, and longitude is first, then latitude * fix list() not [] * fix ATTRIBUTES and URL * coerce all to float, clean up types in header, cast datevec columns to int, create datetimeindex from datevec columns, set index, get errors from json when returning HTTPError Signed-off-by: Mark Mikofski <[email protected]>
|
Bummer, I think the |
|
How's it looking? Ready? |
* other params full name and affiliation are kwargs * set reason to pvlib something as suggested by NREL * catch some errors, like 403 forbidden, are in content, not json, only some errors like bad well known text WKT gemetry are in JSON * use python-2 json decode error * add another test to capture both bad API key and bad WKT * add note on how to register for key, why they need email, and when it would be used, and warning about DEMO_KEY being rate limited Signed-off-by: Mark Mikofski <[email protected]>
|
@cwhanse and @wholmgren not sure what's up with codecov, it won't even let me log in, everything else is green |
| # check errors | ||
| with pytest.raises(HTTPError): | ||
| psm3.get_psm3(LATITUDE, LONGITUDE, api_key='BAD', email=PVLIB_EMAIL) | ||
| with pytest.raises(HTTPError): |
There was a problem hiding this comment.
api_key="BAD" fails because the response is HTTP/1.1 403 Forbidden and there's a long error message in the content, no JSON, that says something like, "please go to NREL developer network and register for a key"- Bristol-UK fails because PSM3 only works in the US, maybe South America, and I think some parts of India perhaps around Chennai? I can confirm - perhaps this should be a warning in the docstring?
perhaps I should add some more comments here to explain these tests - like I did here - for future devs?
There was a problem hiding this comment.
BAD was self explanatory. I don't think we need to test that it fails outside the US. Not our problem.
There was a problem hiding this comment.
PSM3 has data in a lat/long rectangle around the lower 48 states, so there's coverage in northern Mexico for example. But it has a coastline filter and excludes data in the Gulf of Mexico. I think we should test for failure at lat/long where PSM3 does not have data.
There was a problem hiding this comment.
Does PSM3 return a sensible error if lat/lon outside of its area?
There was a problem hiding this comment.
from get_psm3 import get_psm3
import api_keys
MY_KEY = api_keys.get_api_key('nsrdb')
for lat, lon in zip([25, 25, 25], [-125, -124.5, -124]):
try:
meta, data = get_psm3(lat, lon, names='tmy', interval=60,
api_key=MY_KEY)
except Exception as e:
print("{}, {}: ".format(lat, lon), e)
Returns
25, -125: ["The 'wkt' parameter did not match any known data points."]
25, -124.5: Expecting value: line 1 column 1 (char 0)
25, -124: Expecting value: line 1 column 1 (char 0)
The first line is what I expect, the next 2 are json decoding errors that puzzle me.
There was a problem hiding this comment.
Aha, I suspect the 2nd and 3rd messages are empty responses because of the once-per-2second throttle, so raise requests.HTTPError(response.json()['errors']) is throwing the exception about Expecting value:...
Confirmed.
There was a problem hiding this comment.
["The 'wkt' parameter did not match any known data points."]
This could be confusing to pvlib users because we're asking the to specify lat/lon instead of API. So I'm ok with catching this in the function and raising a more descriptive error about lat/lon.
Co-Authored-By: mikofski <[email protected]>
Co-Authored-By: mikofski <[email protected]>
* in raise docstring, change "returns" to "raises", and clarify that either r.json()['errors'] or r.content is the exception arg depending on the type of HTTP error, and give examples * add warning in docstring that data are limited to NSRDB locations * comment on WKT formatting and add TODO to consider making a format_WKT() function in tools.py one day * add response kwarg to HTTPError for better user exception handling * in test_psm3 add comments to clarify the tests for errors and add two more test for bad "names" and interval args
|
I figured out why codecov is complaining, the |
|
@mikofski thanks for looking into the coverage. No need to fix it. |
|
Is it okay? 🚀 I fixed the defaults, but I don't have time to check the variants for "names", has anyone else already done this? Anymore comments? |
|
@cwhanse do you approve of the changes? merge at will 🚀 |
I don't feel comfortable adding that though. I think current handling is sufficient, and I think anything further could and should be handled by the user because it may differ. Maybe you could add that as an issue? My preference is for smaller methods that are supplemented later by more features as needed, versus adding a lot of functionality up front that might not be required. |
initial concept for psm3 reader
docs/sphinx/source/api.rstfor API changes.docs/sphinx/source/whatsnewfile for all changes.