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

Improve docs on Arrow table input #2039

Open
visr opened this issue Feb 4, 2025 · 3 comments
Open

Improve docs on Arrow table input #2039

visr opened this issue Feb 4, 2025 · 3 comments
Labels
documentation Improvements or additions to documentation

Comments

@visr
Copy link
Member

visr commented Feb 4, 2025

What we have
A configuration example with comment: https://ribasim.org/reference/usage.html#configuration-file
GeoPackage database and Arrow tables: https://ribasim.org/reference/usage.html#sec-geopackage
A tip on using Arrow state files: https://ribasim.org/reference/node/basin.html#sec-state

What is missing
An example of how to tell Ribasim Python to write a table to Arrow, like: https://github.com/Deltares/Ribasim/blob/v2025.1.0/python/ribasim_testmodels/ribasim_testmodels/basic.py#L210

And perhaps these sections can reference each other.

@visr visr added the documentation Improvements or additions to documentation label Feb 4, 2025
@github-project-automation github-project-automation bot moved this to To do in Ribasim Feb 4, 2025
@Huite
Copy link
Contributor

Huite commented Feb 4, 2025

I must admit I haven't looked at the arrow input much, but this: model.basin.profile.set_filepath(Path("profile.arrow")) is not very intuitive.

I would prefer a property, basin.to_arrow = True or something.

Ideally, for the model, there's a single switch: e.g. write(arrow=True).

@visr
Copy link
Member Author

visr commented Feb 4, 2025

Yeah the name set_filepath probably only makes sense if you know the internals.

For a Boolean we'd need to make a default mapping from table name Basin / profile to a path e.g. $(input_dir)/Basin/profile.arrow or something like that.

For the database we don't have the freedom in the TOML to select another table in the database, since Basin / profile is the fixed name. For Arrow files we currently do have this option. I imagine it can be useful sometimes to quickly swap out tables by only changing the TOML, but we could consider removing this option since I think it barely gets used so far.

I'd hold off on write(arrow=True) since the downsides are currently no QGIS support #318, and these files are usually not editable.

@Huite
Copy link
Contributor

Huite commented Feb 4, 2025

I'd hold off on write(arrow=True) since the downsides are currently no QGIS support #318, and these files are usually not editable.

You can keep the default value False. For imod-python, we have write(binary=True); the binary stuff isn't readable or editable either. You might argue this is a little bit different, we treat the MODFLOW formats just as an intermediate format (where data goes to die) and the Ribasim input is all nice open formats.

You can then document that the arrow stuff maybe works a little less smooth for now in combination with QGIS. I think it's fine to say that you cannot edit the arrow tables in QGIS. If manual editing is required, the model should be small anyway; in which case it can be stored in a geopackage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: To do
Development

No branches or pull requests

2 participants