Separate UKFetcher into NRFA, SEPA, Environment Agency APIs

[simonmoulds]

The current UKFetcher class uses the Environment Agency Hydrology API, which actually only fetches data for England (i.e. not Scotland, Wales or Northern Ireland). This is a legacy from the RivRetrieve R package.

There are three relevant APIs for the UK:

• Environment Agency (EA) Hydrology API
• Scottish Environment Protection Agency (SEPA) Timeseries API
• National River Flow Archive (NRFA)

The NRFA is the official UK record of river flow, providing access to daily, monthly and peak flow data. It contains data from the Environment Agency (England), SEPA (Scotland), National Resources Wales (Wales) and Department for Infrastructure - Rivers (Northern Ireland), but does not provide access to realtime data. The EA and SEPA APIs provide access to historical and near-realtime data at sub-daily time scales (down to 15 minutes).

One solution is to split the UKFetcher into NRFAFetcher, EAFetcher and SEPAFetcher. This would imply a change in the naming convention adopted more broadly across the project. Another approach would be to use UKFetcher for NRFA data, and create new classes EnglandFetcher and ScotlandFetcher - this would be consistent with the current approach.

This issue somewhat relates to #9, especially for countries with a separate API for archived historical data and realtime data (e.g. Canada).

[simonmoulds]

Continuing the discussion from #38, also relates to #3

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?

This is probably my use case too, but I think there are some applications (e.g. teaching) where this would not happen, and you want to use the package for exploring the available hydrometric data. But if you only want daily min/max values (e.g.) then downloading hourly data is still more effort than you would really want. I agree that providing daily min/max is relatively uncommon (of the APIs we currently cover, Eaufrance, EA, SEPA do this). It's slightly more common to provide data both sub-daily and daily data, so we still need a mechanism for the user to specify whether they want daily or subdaily.

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?

I think your approach of doing this through the variable name, rather than changing the get_data implementation, is reasonable. I would prefer something like discharge_daily_min rather than just daily_min - so it's clear what is being downloaded. In APIs such as Norway (#27) where the same API is used for daily and hourly data, we could follow the same logic - i.e. variable='discharge_hourly', variable='discharge_daily_mean'.

TL;DR - I agree that we try and use the variable argument to specify different time resolutions/aggregation statistics that may be supported by the API, to the extent that we also want to support them through the package.