Skip to content

Conversation

@visr
Copy link
Member

@visr visr commented Sep 30, 2025

This extends NetCDF reading support (#2434), specifically the demand_priority dimension, so these tables:

  • UserDemand / time (demand_priority, node_id, time)
  • LevelDemand / time (demand_priority, node_id, time)
  • FlowDemand / time (demand_priority, node_id, time)

This PR contains some more refactoring of our NetCDF reading code to be able to handle files written by Delft-FEWS. This means e.g. recognizing coordinates by their cf_role or standard_name first, rather than only the variable name. For instance FEWS wouldn't write node_id, but stations with cf_role = timeseries_id.

This still needs more testing, but since I cannot do that now, I made #2617 and propose we add these later.

@visr visr marked this pull request as ready for review October 6, 2025 07:57
@visr visr requested a review from evetion October 6, 2025 07:58
Copy link
Member

@evetion evetion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that 99% of the code here could be upstreamed to a table interface implementation on NCDatasets. Without significant tests, I'm hesitant to merge this, especially given the inner/outer repeats that are prone to bugs.

end

@testitem "netcdf input" begin
# TODO use NCDatasets.ncgen to create Delft-FEWS flavored NetCDF input files
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

@visr visr merged commit ea9b18c into main Oct 7, 2025
11 of 12 checks passed
@visr visr deleted the read-more branch October 7, 2025 11:17
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.

3 participants