pvlib-python
pvlib-python copied to clipboard
sam iotools #1371
adds a sam.py, and pytest
- [X] I think Closes #1371
- [X] I am familiar with the contributing guidelines
- [X] Tests added
- [x] Updates entries in
docs/sphinx/source/reference
for API changes. - [x] Adds description and name entries in the appropriate "what's new" file in
docs/sphinx/source/whatsnew
for 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`
). - [X] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
- [X] Pull request is nearly complete and ready for detailed review.
- [x] Maintainer: Appropriate GitHub Labels (including
remote-data
) and Milestone are assigned to the Pull Request and linked Issue.
@shirubana when sticker turns green we'll approve and run the tests.
@williamhobbs, since you opened #1371, would you be interested in taking a look at this PR from a user's perspective?
@williamhobbs, since you opened #1371, would you be interested in taking a look at this PR from a user's perspective?
Sure! I just ran a test with some data I previously grabbed with pvlib.iotools.read_surfrad
. Here are a few comments:
- The
metadata
dictionary returned bypvlib.iotools.read_surfrad
stores the time zone as"tz": "UTC"
. Two issues come up from that:-
sam.py
is looking for "TZ" instead of "tz". Not sure if this matters, since "tz" doesn't seem to be standardized acrossiotools
, but it might help a bit to be consistent. - NREL SAM needs the time zone as a number that is UTC offset, e.g., 0 for UTC or -6 for MDT. It would be nice if
sam.py
could return an error or warning when passing a time zone string, or even better, convert time zones that might get created by other functions iniotools
into the number fomat SAM is looking for.
-
- When filling in the year so that it starts Jan-1 at 12 AM and ends with the last interval of the calendar year, I typically "roll" the data if it starts mid-year (e.g., if I have data from March 2018 through Feb 2019, move Jan and Feb 2019 to the beginning of the file). In my test case, I pulled data for a full calendar year in UTC (starting at 2019-01-01 00:00 UTC and ending 2019-12-31 23:59 UCT), but after running through
sam.saveSAM_WeatherFile()
, I ended up with a two-year weather file since 2019-01-01 00:00 UCT is 2018-12-31 18:00 CST and it filled in zeros for the rest of 2018. I'm not sure the best way to handle this, but I think the current approach could be confusing. - Naming the function
write_sam()
or similar might better match existing naming conventions, but I'm also not familiar with pvlib function name guidelines. -
sam.saveSAM_WeatherFile()
requires asource
value in the metadata file, but that isn't produced byread_surfrad
and some of the otheriotools
functions. Would it makes sense to have this function default to something, e.g., "pvlib export", if the user doesn't provide an input value?
@williamhobbs do you have an example of your rolling method?
I addressed everything else except that and seems veyr important to do so, and also the timezone string to number function, is a nice add on in general but I probably won't get around to that specifically for this PR.
@williamhobbs I added a rolling method. Whenever you get an opporutnity can you re-try your surfrad data?
Thanks!
@shirubana The rolling seems to work based on a quick test.
I'm getting the time zone in the output file as "UTC" instead of "0". I think users would expect the time zone in the metadata returned from read_surfrad to work, or to get an error/warning.
I've also just realized that it would be helpful to be able to split out hourly averaging from the other standardSAM behaviors (skipping leap day and making the file start with Jan 1 12:00 AM), since SAM does not require hour intervals but does require the other items. I would suggest either:
- removing the resampling (
_averageSAMStyle
) or - creating an additional input that is either a boolean for hourly averaging or something that lets you pass an averaging interval in minutes.