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.

Sign up for GitHub

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

Jump to bottom

validation of inputs #24

Closed
4 tasks
gijsber opened this issue Nov 28, 2022 · 6 comments · Fixed by #43 or #166
Closed
4 tasks

validation of inputs #24

gijsber opened this issue Nov 28, 2022 · 6 comments · Fixed by #43 or #166
Assignees
@evetion
Milestone
Sprint 23.3 incre...

Comments

@gijsber
Copy link
Contributor

gijsber commented Nov 28, 2022 •
edited by visr
Loading

Enable validation of input:

  • rules
  • API
  • code implementation
  • documentation
@gijsber gijsber added this to Ribasim Nov 28, 2022
@gijsber gijsber moved this to 🔲 Todo in Ribasim Nov 28, 2022
@gijsber gijsber added this to the EndOf22 package milestone Nov 28, 2022
@evetion evetion moved this from 🔲 Todo to 🏗 In progress in Ribasim Dec 5, 2022
@evetion
Copy link
Member

evetion commented Dec 5, 2022

So implement on both levels:

  • Internal data structures
  • But also in our "blessed" (GraphML?) external format

@visr
Copy link
Member

visr commented Dec 22, 2022

This looks interesting for validating Arrow tables: https://github.com/beacon-biosignals/Legolas.jl/. It is used in https://github.com/beacon-biosignals/LegolasFlux.jl/.

@evetion evetion mentioned this issue Jan 19, 2023
Merged
@visr visr closed this as completed in #43 Jan 26, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Ribasim Jan 26, 2023
@evetion evetion reopened this Feb 27, 2023
@github-project-automation github-project-automation bot moved this from ✅ Done to 🏗 In progress in Ribasim Feb 27, 2023
@evetion
Copy link
Member

evetion commented Feb 27, 2023

  • Check whether extra keys will throw an error

@gijsber
Copy link
Contributor Author

gijsber commented Mar 6, 2023 •
edited by visr
Loading

for this moment we can have data struct duplications (julia, plugin, python)
if we introduce rules, you want to have it better organized
at this moment keep validation in julia ~3d (complexity we may need to touch the structs that affect computation)

For future revisit if we want to move to Rust-CLI (#90) doing validation before handing over to julia. This could be used by python and plugin as well.

@evetion
Copy link
Member

evetion commented Mar 16, 2023

Ok, so ideally, we generate a schema (jsonschema) from a single source, and use that in all other code. For me, that looks like:

Julia -> Jsonschema -> Python / QGIS

The main reason is because Julia has to be able to compute with the input, so it has to be compatible there.

One can generate Pydantic BaseModels in Python from jsonschemas. These can be used in Pandera as well, although it might be slower. There's an open issue for Pandera to directly generate Pandera models from jsonschema, feel free to upvote it.

@evetion
Copy link
Member

evetion commented Apr 6, 2023 •
edited
Loading

  • Make JSONSchema (ideally automatically)
  • Implement use of JSONSchema in Python derivatives
  • Make API for new nodetypes, so everything can be updated automatically
  • (optional, new issue) Investigate possibility of validating possible combinations of nodetypes

@evetion evetion mentioned this issue Apr 16, 2023
Merged
@evetion evetion moved this from 🏗 In progress to 👀 In review in Ribasim Apr 17, 2023
@visr visr closed this as completed in #166 Apr 17, 2023
visr added a commit that referenced this issue Apr 17, 2023
@evetion @visr
Enable validation by adding schemas for all IO layer rows. Added / st…
6c115a3 
…atic postfix. (#166)

Fixes #24 
Fixes #144 

Now one only needs to specify a new schema and record in
`validation.jl`, from which both the Config as the validation will be
triggered. This is completely discoupled from the compute types, the
connection is only made on the call to load_data like `data =
load_data(db, config, TabulatedRatingCurveStaticV1)`.

Note that I've taken the liberty to already append `static` to all base
tables, so this requires new geopackages. I've also inversed the
`Config` to have a [basin] key with forcing underneath instead of
[forcing] with basin underneath.

One can generate jsonschema's with `julia --project=core
scripts/gen_schema.jl`. These end up in the docs/schema folder, which
will then be live on the website as well. In another PR I will automate
the Python and QGIS code based on these schemas. Incremental changes to
validation can be made now by a single line change, like `node_id::Int =
clamp(node_id, 0, typemax(Int))`.

---------

Co-authored-by: Martijn Visser <[email protected]>
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Ribasim Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@evetion evetion

Labels
None yet
Projects
Ribasim
Archived in project
Milestone
Sprint 23.3 increment
3 participants
@visr @evetion @gijsber