Skip to content

Variable naming refactor, extending fetchers, fixing stage data retrieval in france. - #46

Merged
kratzert merged 4 commits into
mainfrom
refactor
Oct 17, 2025
Merged

Variable naming refactor, extending fetchers, fixing stage data retrieval in france.#46
kratzert merged 4 commits into
mainfrom
refactor

Conversation

@kratzert

@kratzert kratzert commented Oct 16, 2025

Copy link
Copy Markdown
Owner

Thiis is a bit of a hail marry pull requests that does many things at once. It wasn't planned like this, but while refactoring the names according to the discussion in #3 I also noticed:

  • A bug in the France fetcher for stage data (we had the wrong column name)
  • How easy it is to extend the USA and South Africa fetcher. For the USA, we now support daily stage mean/max/min, daily discharge, and instantaneous discharge and stage. They call it instantaneous, but it is at a regular 15 minute interval. Not sure if we want to call in "discharge_instantaneous" or "dischage_15min" (as e.g. for UK-EA).
  • Removed aggregation to daily mean from hourly stage data in UK-EA after our discussion on not doing aggregations ourself.
  • Haven't yet updated the UK-EA fetcher, related to UKFetcher renaming and extension #41 for the other var names as I couldn't find a list quickly.
  • There is also more data available in Australia, see Extend Australia fetcher #45 that I haven't added here.

Fixes #3

@simonmoulds Before I merge this, we should maybe discuss if we need a variable name with "instantaneous" and "15min". From what I gather, everyone who provides "15min" data, calls this "instantaneous" e.g. USGS, UK-EA, and then in South Africa, instantaneous are random timesteps. The point here is that from what I can tell, there are no aggregations happening over 15min data. So everything can be considered instantaneous that is at this temporal interval. And tbh, I think having e.g. the sub-hourly data from South-Africa under a different name, just because they don't seem to log in a regular interval, than other countries, sounds weird. I don't have a strong opinion though.

This was referenced Oct 16, 2025
@simonmoulds

Copy link
Copy Markdown
Collaborator

Yeah OK - so one approach could be to only provide the time resolution and aggregation function for values that have been resampled from the instantaneous data. You're right - I think it's unlikely that 15 minute data has been aggregated, although it's possible of course (especially nowadays, with edge computing and when the data storage cost of having very high resolution readings is negligible). Hourly is getting quite coarse for instantaneous measurements - OK for bigger rivers but for smaller rivers I should think most hydrometric agencies would take readings at a finer time resolution, because a flood peak could be missed.

This also relates back to our discussion somewhere else about the gauging-flow and gauging-stage values from NRFA, which are the instantaneous spot gaugings from the measuring agencies and also constitute an irregular time series (albeit with only a few hundred over decades).

The naming convention we have currently adopted indicates an instantaneous value by not providing the aggregation function - e.g. discharge_15min implies instantaneous values at regular 15 minute intervals. For the SA case I would do something like discharge_irregular to indicate irregular instantaneous measurements, and document that these are at subdaily resolution. We could also do discharge_subdaily-irregular or similar to differentiate subdaily irregular measurements.

@kratzert

Copy link
Copy Markdown
Owner Author

Some APIs seem to related to this data simply as "discharge_point", which could be used for both, 15min and irregular. I want to prevent that we start adding different string combinations for every other fetcher, because in the end you have thousands of different variable names for discharge. To me, it should be clear from the name if the data is a) aggregated and b) if it is at a fine or coarse resolution. If it is 15min/irregular, I know that I have to resample it to daily means before I can combine it with discharge_daily. But if it is 15min or irregular, is not like that makes a huge difference for me? The data will be stored in separate files anyway, no?

@simonmoulds

simonmoulds commented Oct 17, 2025

Copy link
Copy Markdown
Collaborator

OK - I agree we want to limit the string combinations. So are you suggesting something like discharge_instantaneous for any instantaneous measurement (regular or irregular), and only indicate the time resolution for aggregated variables (discharge_daily_mean)? That sounds reasonable to me...

@kratzert

Copy link
Copy Markdown
Owner Author

Yes, I was thinking along those lines. If some data provider would have hourly discharge, then this would probably be discharge_hourly_mean but point/instantaneous and 15min data seems to be equivalent (besides the difference in temporal regularity).

@simonmoulds

Copy link
Copy Markdown
Collaborator

OK - sounds good to me.

@kratzert
kratzert merged commit 393e0d8 into main Oct 17, 2025
3 checks passed
kratzert added a commit that referenced this pull request Oct 17, 2025
…eval in france. (#46)

* get_data now returns time indexed dataframes

* Refactor: New names, Extended USA and South Africa, Fixed France

* Change 15min name to instantaneous
@kratzert kratzert mentioned this pull request Oct 17, 2025
@kratzert
kratzert deleted the refactor branch October 17, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for downloading data the native temporal resolution

2 participants