Add UK-NRFA fetcher, #34 - #38
Conversation
|
Maybe And the dataframe is indexed by "gauge_id" (i.e. constants.GAUGE_ID). Related to #15 |
I agree. I think having location and river names is essential to make sure we can filter potential duplicates in the future. |
|
I would suggest including gauged daily flow(gdf), gauged monthly flow (gmf), catchment daily rainfall (cdr) and catchment monthly rainfall (cmr). I don't think that naturalised flow is available for many catchments and I don't really know how this is estimated, so I would suggest leaving for now. For global variable names it probably depends a bit on the style we want to adopt. It could be discharge and precipitation, or Q and P (note: I think in this instance rainfall is synonymous with precipitation). However, as we are also thinking about retrieving data at different time resolutions (i.e. #3), do we want to apply a naming convention that details the resolution and aggregation method? SEPA do something like: '15minute' [i.e. instantaneous], 'CalendarYear.Mean', 'Month.Min', for example. |
|
For the metadata columns, I think it makes sense to choose a subset which will be provided by the vast majority of providers. In OHDB [@thiagovmdon - this is a streamflow database I compiled with colleagues at Oxford], we had: We found that providing information about the subsource was helpful when dealing with providers like GRDC or NRFA, which pull data from other sources (e.g. EA, SEPA in the case of NRFA). Start and end dates could either be taken from the original metadata, or calculated from the timeseries directly. |
|
For discussion on attribute names, see #39 For NRFA specific data vars:
Not entirely sure what I prefer |
|
I should think the I agree the time resolution can be inferred from the timeseries itself. But the method of aggregation can't (e.g. mean, min, max, instantaneous). So we might want to document this information somehow. It could be done in a separate column in the timeseries dataframe, or through a naming convention. |
Perhaps also including level would be better? Also, is there temperature for those UK stations? if yes, I would also be in favor of including. |
|
They don't have stage/level as time series. At least not a daily time series. For precipitation, should the name be |
Okay, still not sure what to do with this. I don't want to over-complicate things. The main purpose of the library is to facilitate access to data from the provider. We return time-indexed data that can be saved to csv/netCDF/zarr and can be processed with any of the standard libraries (pandas, xarray). I tend a little bit towards not even supporting custom aggregations to different resolutions, at least not at the fetcher level. Because this is not really the task of a class that is responsible for fetching data. We can add utilities to resample the data, but with pandas and a time-indexed dataframe it is literally as simple as df_monthly_mean = df.resample('M').mean()
df_monthly_max = df.resample('M').max()
df_monthly_min = df.resample('M').min() Not really sure if we need to provide a function to wrap these one-lines or if we can assume that someone who bulk-downloads streamflow data is able to resample data. The only thing I see is that it might make sense to separate between daily values and hourly (or instantaneous). I would say most modeling tasks are usually done with daily data, but I am personally highly interested in sub-daily data. We could give it a different variable name (not sure if we then need multiple for e.g. hourly and 15-min or whatever frequencies we encounter), or if we still return this data under the variable "discharge" and there is some other mechanism to say I want daily, or whatever sub-daily data is available. |
Yes, I think so |
I agree with the point about not needing to provide custom aggregations - that is best left to the user and it is straightforward. I think we should be returning data that is as close to that received from the API as possible, since that is the main purpose of the library. The only difficulty is when the API allows you to request aggregated data (e.g. SEPA allows you to request the daily mean/min/max streamflow). It is much quicker for the user to request the daily aggregations than request the 15-minute instantaneous values only to aggregate them themselves. What about adding an argument (e.g. |
I also agree with not doing resamples ourselves, and that the final df will speak by itself. I think this is a nice suggestion @simonmoulds. Just one query: is instantaneous the best "naming" when we have like 15-min, hourly etc? "subdaily" would not make more sense? (talking as a general for all countries). So, sub daily (or any other naming we agree on) would be the highest resolution available, and daily (mean, max, min). No resamplings |
Well, I partially agree. Sure, it is faster to get daily min/max directly if that is provided rather than getting hourly and aggregating by min/max operation. That being said, it is the first time I hear some data provider provides something else than daily mean. And from my personal understanding/use of this library, I would not assume people go and download the hourly data over and over again. Instead you let this run once and then save the downloaded data, after which it doesn't really matter? That being said, I also don't have strong opinions about including daily_min/daily_max as two additional variables (I think in that case it is easier to just have two additional variable names that in that case might only be supported by one fetcher) rather than changing the get_data implementation of all other fetchers. Sounds reasonable? But we can discuss more on this in #32 For this PR, I will stick to daily discharge and daily catchment precipitation for now (still need to push that) and will also start using the global attribute names added in #39 for the metadata. |
* Add a few attribute names * Some more attribute names
Todo before merge
gdf). Which other variables do we want to include, see https://nrfaapps.ceh.ac.uk/nrfa/nrfa-api.html#parameter-data-type and what are the global names we want to use for them?@simonmoulds any thoughts on the first point?
@simonmoulds and @thiagovmdon any thoughts on the second point?