Add getter/parser for PVGIS hourly-radiation #1186
Conversation
|
@mikofski @wholmgren Should the tests for the hourly radiation functions just mimick those for the TMY functions? |
|
That's probably okay. I think the goals for testing should be, "have I sent the request correctly?" "Is the response what I expected?" and "did it get parsed right?" Make sure to mark api calls with |
Change lat/lon to latitude/longitude and changed startyear/endyear to start/end. Also, changed meta to metadata
This reverts commit d7deb80.
AdamRJensen
force-pushed
the
pvgis_hourly
branch
from
99da596 to
b416f62
Compare
|
@kanderso-nrel @mikofski The basic format is essentially the same as the csv option, but without a header or metadata. Is there any interest in supporting this option? Note that for PVGIS TMY the basic format has a header, which it is NOT supposed to have according to the development team behind PVGIS. |
AdamRJensen
force-pushed
the
pvgis_hourly
branch
from
e4aa714 to
b416f62
Compare
This reverts commit b416f62.
There was a problem hiding this comment.
Looking good @AdamRJensen, some minor comments below. I think this is ready for a detailed look from someone else now, maybe @mikofski for familiarity with the original PVGIS functions? Though obviously anyone is welcome to review.
|
|
||
| Parameters | ||
| ---------- | ||
| filename : str, pathlib.Path, or file-like buffer |
There was a problem hiding this comment.
Without thinking too much about it, one clean approach might be a decorator that does something like (a much simpler version of) pandas's _get_filepath_or_buffer: https://github.com/pandas-dev/pandas/blob/master/pandas/io/common.py#L226
There was a problem hiding this comment.
A handful of minor and mostly optional things... Thanks @AdamRJensen! Another maintainer can merge when ready.
Also remove instances of using inplace
There was a problem hiding this comment.
LGTM thanks for seeing this through @AdamRJensen! Two very minor points below, then I promise I'll hit the merge button.
docs/sphinx/source/api.rstfor 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`).Add getter/parser for PVGIS hourly-radiation