Ribasim Python: Add-API #1110
Merged
Ribasim Python: Add-API #1110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 12, 2024
visr
added a commit
that referenced
this pull request
Feb 12, 2024
Needed for #1110. Before node IDs were globally unique. If you had the ID, you could look up the type in the Node table. When designing #1110 we came to the conclusion that this was not the right choice. It requires you to be aware of the IDs that are used throughout the model, whereas with this you only need to make sure that `Pump #5` doesn't already exist. Most of the updates in this PR were to correct the tests. Some of the code and error messages became easier to read. If we talk about a node then we always know what the type is, there is no need to look it up first. Most tables stay the same, e.g. if you have a `Terminal / static` table with a `node_id` Int column, you know that this refers to a Terminal NodeID. Only when connecting to other nodes do we need to specify the type next to the ID. So the `Edge` table now gets `from_node_type` next to `from_node_id`, the `PidControl / static` gets `listen_node_type` next to `listen_node_id`, etc. These extra columns are currently automatically filled in by Ribasim-Python on model save, hence they don't require changing the test models. In terms of implementation, this basically adds the `type` field to `NodeID` and fixes the resulting errors. ```julia struct NodeID type::NodeType.T value::Int end ``` It does not yet test if models with the same node IDs (`Pump #1` and `Basin #1`) work, but this is hard to do right now with Ribasim Python, so best left for a later moment. --------- Co-authored-by: Hofer-Julian <[email protected]>
This was referenced Feb 14, 2024
Closed
Hofer-Julian
force-pushed
the
add-api
branch
3 times, most recently
from
b33839b to
5bc0a91
Compare
Hofer-Julian
force-pushed
the
add-api
branch
5 times, most recently
from
05a171b to
e4b2a09
Compare
Validate during initialization Improve Node representation Adapt type hint Minimal version of `Model` Add existing methods to `Model` Continue Fix remaining problems Adapt codegen Add launch config for playground Generate schemas Remove `Network` class Stop avoiding database locking Allow incrementally adding nodes Move to add api only Add `EdgeData` Use explicit `Node` and `Point` class as input Adapt the Edges class Fix `Edge.add` Share code between Basin and Basins Avoid pydantic warnings Add plotting functionality Add more node types Migrate tabulatedratingcurve Update remaining nodes Simplify Edge API Port first part of examples Add ribasim_version Let tables be appended and continue on example Migrate first example Rename Update examples Move to new API Add remaining nodes Adapt yet another example Rename `add` to `model` and port another example Pre-commit
|
For updating |
visr
added a commit
that referenced
this pull request
Mar 8, 2024
) @JvanHouwelingen noticed that we don't create a `node_id` column instead of `fid`. To be able to edit tables in QGIS we always need `fid`, but its value no longer matters, so leaving it to autogenerate is normally best. This removes `explode_and_connect` to automatically fill in part of the Edge attributes when creating a new geometry. It was no longer correct due to the schema updates, so better to remove it for now, and accept that users have to both draw the geometry and fill in the correct attributes. An issue with the desired behavior is already written in #352. In short, `derive_connectivity` would still be nice to have, but not `explode`. - add `node_id` to the Node schema - use `node_id` in the Node label - add `from_node_type` and `to_node_type` to the Edge schema - keep showing fid in the Edge label to be able to associate edge_id in timeseries widget - use a dropdown selector for the Node and Edge editor widget, see screenshot below - update schemas with `listen_node_type` as in #1110  Edit: with the updates from Huite this fixes #352 --------- Co-authored-by: Huite Bootsma <[email protected]>
Hofer-Julian
pushed a commit
that referenced
this pull request
Mar 11, 2024
) @JvanHouwelingen noticed that we don't create a `node_id` column instead of `fid`. To be able to edit tables in QGIS we always need `fid`, but its value no longer matters, so leaving it to autogenerate is normally best. This removes `explode_and_connect` to automatically fill in part of the Edge attributes when creating a new geometry. It was no longer correct due to the schema updates, so better to remove it for now, and accept that users have to both draw the geometry and fill in the correct attributes. An issue with the desired behavior is already written in not `explode`. - add `node_id` to the Node schema - use `node_id` in the Node label - add `from_node_type` and `to_node_type` to the Edge schema - keep showing fid in the Edge label to be able to associate edge_id in timeseries widget - use a dropdown selector for the Node and Edge editor widget, see screenshot below - update schemas with `listen_node_type` as in #1110  Edit: with the updates from Huite this fixes #352 --------- Co-authored-by: Huite Bootsma <[email protected]>
visr
approved these changes
Mar 12, 2024
This was referenced Mar 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #760
Fixes #252
Follow ups: