Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ribasim Python: stop exposing Ribasim.Node and node IDs to users #252

Closed
Hofer-Julian opened this issue May 30, 2023 · 2 comments · Fixed by #1110
Closed

Ribasim Python: stop exposing Ribasim.Node and node IDs to users #252

Hofer-Julian opened this issue May 30, 2023 · 2 comments · Fixed by #1110
Labels
python Relates to one of the Ribasim python packages

Comments

@Hofer-Julian
Copy link
Contributor

Thanks to @SouthEndMusic's work, users will soon have fewer chances to mess up their model input when using Ribasim Python.
#245

What would be even better, if users wouldn't have to think about node IDs at all. They would create specific node types with the corresponding geometries attached. When new nodes are created that way, they would access a singleton allocator which returns new and unique node IDs.

For creating edges, the interfaces might change like this:

- from_id = np.array(
-    [1, 2], dtype=np.int64
-)
- to_id = np.array(
-    [2, 3], dtype=np.int64
-)

+ from_id = [basin.nodes[0], linear_resistance.nodes[0]]
+ to_id = [linear_resisitance.nodes[0], basin.nodes[1]]
@Hofer-Julian Hofer-Julian added the python Relates to one of the Ribasim python packages label May 30, 2023
@visr
Copy link
Member

visr commented Aug 22, 2023

#446 introduces a get_node_ids_and_types function that addresses a part of this. This probably needs to be removed/refined later. Right now it is guessing available tables for node types, ["static", "time", "condition"].

https://github.com/Deltares/Ribasim/pull/446/files#diff-1781035826cbdb5bd206a7c07398ee9d29ad5c34fa4312b6a0b07db7e154d3ab

@visr visr mentioned this issue Sep 12, 2023
@visr
Copy link
Member

visr commented Sep 18, 2023

Fixing this would probably also fix #212, since you would not pass the Node table anymore so there is no way it cannot match.

evetion added a commit that referenced this issue Nov 14, 2023
Fixes #512
Groundwork for #630 & #252

The code has become much simpler in some places (e.g. write_toml).

```python
from ribasim import Model

m = Model(filepath="generated_testmodels/basic/ribasim.toml")
m.database.node.df # Node table
m.basin.static.df # BasinStatc table
m.write("test")
```

### Some notes:
- The config.py file cannot be autogenerated anymore. The schemas still
can, but I disabled it for now to be sure (some imports error).

### Changes:
I created new (parent) classes:
- BaseModel, from Pydantic, with our own config
- FileModel, like hydrolib-core (but now Pydantic v2), which can take a
single filepath for initilization. Models who inherit require defining
_load and _save, dealing with that filepath.
- NodeModel, a class for nodes (where `add` will be).
- TableModel, a class to read/write individual tables from/to
gpkg/arrow.
- SpatialTableModel, inherits TableModel, but reads/writes spatial
stuff.

I changed:
- the Model class to be a carbon copy of Config (which has been
deleted), so it mirrors the toml.
- in turn this created a `database` NodeModel class (reflecting the
field in the toml), with only Node and Edge underneath.
- the NodeModel classes Basin from their node_type version to the one in
Config, and set the type of the underlying table with a TypeVar like so:
```python
class Terminal(NodeModel):
    static: TableModel[TerminalStaticSchema]
```

### Yet to do:
- [x] Update tests
- [x] Fix sort! rules
- [x] Delete node_types folder
- [x] Link schemas to their Pydantic class
(TableModel[TerminalStaticSchema] => TerminalStatic)

---------

Co-authored-by: Martijn Visser <[email protected]>
Co-authored-by: Hofer-Julian <[email protected]>
Co-authored-by: Hofer-Julian <[email protected]>
visr added a commit that referenced this issue Nov 14, 2023
Fixes #512
Groundwork for #630 & #252

The code has become much simpler in some places (e.g. write_toml).

```python
from ribasim import Model

m = Model(filepath="generated_testmodels/basic/ribasim.toml")
m.database.node.df # Node table
m.basin.static.df # BasinStatc table
m.write("test")
```

### Some notes:
- The config.py file cannot be autogenerated anymore. The schemas still
can, but I disabled it for now to be sure (some imports error).

### Changes:
I created new (parent) classes:
- BaseModel, from Pydantic, with our own config
- FileModel, like hydrolib-core (but now Pydantic v2), which can take a
single filepath for initilization. Models who inherit require defining
_load and _save, dealing with that filepath.
- NodeModel, a class for nodes (where `add` will be).
- TableModel, a class to read/write individual tables from/to
gpkg/arrow.
- SpatialTableModel, inherits TableModel, but reads/writes spatial
stuff.

I changed:
- the Model class to be a carbon copy of Config (which has been
deleted), so it mirrors the toml.
- in turn this created a `database` NodeModel class (reflecting the
field in the toml), with only Node and Edge underneath.
- the NodeModel classes Basin from their node_type version to the one in
Config, and set the type of the underlying table with a TypeVar like so:
```python
class Terminal(NodeModel):
    static: TableModel[TerminalStaticSchema]
```

### Yet to do:
- [x] Update tests
- [x] Fix sort! rules
- [x] Delete node_types folder
- [x] Link schemas to their Pydantic class
(TableModel[TerminalStaticSchema] => TerminalStatic)

---------

Co-authored-by: Martijn Visser <[email protected]>
Co-authored-by: Hofer-Julian <[email protected]>
Co-authored-by: Hofer-Julian <[email protected]>
@visr visr closed this as completed in 006ff90 Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Relates to one of the Ribasim python packages
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants