Add retrieval functions for daily precip, snow, and temperature data from NOAA RCC ACIS #1767

kandersolar · 2023-06-10T00:25:58Z

Closes get PRISM rainfall data #1293
I am familiar with the contributing guidelines
Tests added
Updates entries in docs/sphinx/source/reference for API changes.
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`).
New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
Pull request is nearly complete and ready for detailed review.
Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

ACIS provides precipitation estimates from PRISM, plus three other gridded datasets. No registration needed.

ACIS has another API endpoint for station-recorded (not gridded) data that supplies snowfall, among other variables. ~~Possible future PR for that.~~ now included in this PR

kandersolar · 2023-06-10T01:07:03Z

Test failures are something to do with forecasts which I'm not excited about debugging given #1766.

AdamRJensen

Awesome contribution!

pvlib/iotools/acis.py

kandersolar · 2023-06-15T18:50:35Z

After further consideration, I think there are enough differences between datasets that it makes sense to rewrite this PR so that there is a 1:1 correspondence between get_ functions here and datasets in the API. Otherwise the code becomes a mess trying to handle all the datasets in a single function. Marking this PR back to draft for now.

kandersolar · 2023-06-16T17:03:37Z

@AdamRJensen this is ready for another look. In addition to splitting up the function as mentioned above, I also added functions for the weather station dataset.

A discussion point: currently, get_acis_station_data returns precipitation in mm but snow in cm, for consistency with relevant PV models (pvlib.soiling takes rainfall in mm; pvlib.snow takes snowfall in cm). Is that the right choice?

Test failures are pvlib.forecast and can be ignored here.

AdamRJensen

I'll play more around with the functions in the next couple of days.

My main concern is that it is a lot of functions (clutter) compared to the previous state of the PR which had it combined into one function. But it looks solid!

pvlib/iotools/acis.py

AdamRJensen · 2023-06-25T21:06:57Z

pvlib/iotools/acis.py

+    # time series names
+    'pcpn': 'precipitation',
+    'maxt': 'temp_air_max',
+    'avgt': 'temp_air_average',


Perhaps remove _average if this is almost always the temperature of interest for PV applications? For example, the bsrn function doesn't append _average but does append _min and _max.

I lean towards keeping the _average suffix here. Daily values seem different enough from the higher-resolution values that we call temp_air that consistency with the other names in this dict seems more important to me. It doesn't make sense to feed these values into ModelChain.run_model, for example.

pvlib/iotools/acis.py

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

kandersolar

Thanks @AdamRJensen!

My main concern is that it is a lot of functions (clutter) compared to the previous state of the PR which had it combined into one function.

Yes, it does add a bit of clutter to the API listing. But there are enough differences between the datasets that I thought splitting it up would make it easier to use and more straightforward to maintain, which seemed more important.

Any thoughts on this question from #1767 (comment)?

A discussion point: currently, get_acis_station_data returns precipitation in mm but snow in cm, for consistency with relevant PV models (pvlib.soiling takes rainfall in mm; pvlib.snow takes snowfall in cm). Is that the right choice?

kandersolar · 2023-06-27T14:00:30Z

pvlib/iotools/acis.py

+    # time series names
+    'pcpn': 'precipitation',
+    'maxt': 'temp_air_max',
+    'avgt': 'temp_air_average',


I lean towards keeping the _average suffix here. Daily values seem different enough from the higher-resolution values that we call temp_air that consistency with the other names in this dict seems more important to me. It doesn't make sense to feed these values into ModelChain.run_model, for example.

pvlib/iotools/acis.py

wholmgren

thanks @kandersolar - looking forward to using this!

As for snow units, I think I'd rather keep them in mm here and make other functions consistent with that. But ask me again tomorrow and I might have a different opinion.

pvlib/tests/iotools/test_acis.py

pvlib/iotools/acis.py

AdamRJensen · 2023-06-28T11:24:05Z

As for snow units, I think I'd rather keep them in mm here and make other functions consistent with that. But ask me again tomorrow and I might have a different opinion.

I think I agree with Will - we should avoid having too many different units.

kandersolar · 2023-06-29T19:55:42Z

See a bit of discussion in #1792 about snow units. Keeping them as cm seems like the best option to me, at least for now. I think that was the last open item here, so in it goes!

kandersolar added 6 commits June 9, 2023 20:10

create acis.py and add get_acis_precipitation function

3f03323

add tests

94a1e90

sphinx

99fc6b5

tweak

9bcac51

better tests

14dd598

better whatsnew

60fb8ee

kandersolar added enhancement io remote-data triggers --remote-data pytests labels Jun 10, 2023

kandersolar added this to the 0.10.0 milestone Jun 10, 2023

kandersolar requested a review from AdamRJensen June 10, 2023 00:25

import in iotools/init

2719b83

kandersolar mentioned this pull request Jun 10, 2023

v0.10.0 release plan #1735

Closed

kandersolar added remote-data triggers --remote-data pytests and removed remote-data triggers --remote-data pytests labels Jun 10, 2023

fix all these broken things

167d49b

kandersolar added remote-data triggers --remote-data pytests and removed remote-data triggers --remote-data pytests labels Jun 10, 2023

AdamRJensen reviewed Jun 14, 2023

View reviewed changes

kandersolar added 2 commits June 15, 2023 09:35

edits from review

52b4d84

correct PR number

de6333b

kandersolar marked this pull request as draft June 15, 2023 18:51

split up code to be one function per dataset; add station functions

3201413

kandersolar added remote-data triggers --remote-data pytests and removed remote-data triggers --remote-data pytests labels Jun 16, 2023

kandersolar changed the title ~~Add retrieval function for gridded precipitation data from NOAA RCC ACIS~~ Add retrieval functions for precip, snow, temperaturedata from NOAA RCC ACIS Jun 16, 2023

fix tests

7724618

kandersolar removed the remote-data triggers --remote-data pytests label Jun 16, 2023

kandersolar added the remote-data triggers --remote-data pytests label Jun 16, 2023

kandersolar marked this pull request as ready for review June 16, 2023 16:56

kandersolar changed the title ~~Add retrieval functions for precip, snow, temperaturedata from NOAA RCC ACIS~~ Add retrieval functions for daily precip, snow, and temperature data from NOAA RCC ACIS Jun 17, 2023

AdamRJensen reviewed Jun 25, 2023

View reviewed changes

kandersolar mentioned this pull request Jun 27, 2023

Add get_srml iotools function; deprecate read_srml_month_from_solardat #1779

Merged

7 tasks

kandersolar and others added 2 commits June 27, 2023 09:49

Apply suggestions from code review

12cd83a

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

more from review

16fbafd

kandersolar commented Jun 27, 2023

View reviewed changes

wholmgren approved these changes Jun 27, 2023

View reviewed changes

pvlib/tests/iotools/test_acis.py Outdated Show resolved Hide resolved

pvlib/iotools/acis.py Outdated Show resolved Hide resolved

pvlib/iotools/acis.py Outdated Show resolved Hide resolved

kandersolar added 3 commits June 27, 2023 14:24

add degree days to df descriptions

fa85d3a

add degree day columns to variable map

9cbe851

fix valid daterange test oversight

30b3703

kandersolar added remote-data triggers --remote-data pytests and removed remote-data triggers --remote-data pytests labels Jun 27, 2023

cm -> mm for snow variables

3b48739

kandersolar mentioned this pull request Jun 28, 2023

Consider changing pvlib.snow to expect precipitation in mm instead of cm #1792

Closed

revert back to cm for snow

a03a774

kandersolar added remote-data triggers --remote-data pytests and removed remote-data triggers --remote-data pytests labels Jun 29, 2023

kandersolar merged commit 9be4487 into pvlib:main Jun 29, 2023
33 of 35 checks passed

kandersolar deleted the prism branch June 29, 2023 19:55

AdamRJensen mentioned this pull request Nov 22, 2023

NOAA NCDC global summary of the day GSOD for rainfall in iotools #668

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add retrieval functions for daily precip, snow, and temperature data from NOAA RCC ACIS #1767

Add retrieval functions for daily precip, snow, and temperature data from NOAA RCC ACIS #1767

kandersolar commented Jun 10, 2023 •

edited

kandersolar commented Jun 10, 2023

AdamRJensen left a comment

kandersolar commented Jun 15, 2023

kandersolar commented Jun 16, 2023

AdamRJensen left a comment

AdamRJensen Jun 25, 2023

kandersolar Jun 27, 2023

kandersolar left a comment

kandersolar Jun 27, 2023

wholmgren left a comment

AdamRJensen commented Jun 28, 2023

kandersolar commented Jun 29, 2023

Add retrieval functions for daily precip, snow, and temperature data from NOAA RCC ACIS #1767

Add retrieval functions for daily precip, snow, and temperature data from NOAA RCC ACIS #1767

Conversation

kandersolar commented Jun 10, 2023 • edited

kandersolar commented Jun 10, 2023

AdamRJensen left a comment

Choose a reason for hiding this comment

kandersolar commented Jun 15, 2023

kandersolar commented Jun 16, 2023

AdamRJensen left a comment

Choose a reason for hiding this comment

AdamRJensen Jun 25, 2023

Choose a reason for hiding this comment

kandersolar Jun 27, 2023

Choose a reason for hiding this comment

kandersolar left a comment

Choose a reason for hiding this comment

kandersolar Jun 27, 2023

Choose a reason for hiding this comment

wholmgren left a comment

Choose a reason for hiding this comment

AdamRJensen commented Jun 28, 2023

kandersolar commented Jun 29, 2023

kandersolar commented Jun 10, 2023 •

edited