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

ENH: Add pydantic meta schema #409

Merged
merged 25 commits into from
Jan 25, 2024
Merged

ENH: Add pydantic meta schema #409

merged 25 commits into from
Jan 25, 2024

Conversation

janbjorge
Copy link
Contributor

@janbjorge janbjorge commented Jan 9, 2024

Addresses #423

This PR is does not change any logic, it just adds a Pydantic model to the repository. There a few convenience tools as well, that i used while dev. the pydantic model.

@janbjorge janbjorge self-assigned this Jan 9, 2024
@janbjorge janbjorge changed the title experimental: Build pydantic from schema Experimental: Build pydantic from schema Jan 9, 2024
@janbjorge
Copy link
Contributor Author

janbjorge commented Jan 18, 2024

In its current state it fully runs boom.py without any validation errors.

A few assumtions has been made in order to attempt to clean up the schema abit:

  • None / Null is interpreted as falsy. If the the annotation is Optional[bool] this will be converted to bool, with default false. The same approach can be done for lists. Where an Optional[list] will default to an empty list removing the need for an optional
  • For ints and floats the default will be zero. This might not be correct for all fields, but i think for a few of them it makes perfect sense. Ex. ncol.

@janbjorge janbjorge changed the title Experimental: Build pydantic from schema ENH: Add pydantic meta schema Jan 18, 2024
@janbjorge janbjorge marked this pull request as ready for review January 25, 2024 13:52
@janbjorge janbjorge requested review from perolavsvendsen and removed request for perolavsvendsen January 25, 2024 13:52
@janbjorge janbjorge requested review from mferrera, jcrivenaes and perolavsvendsen and removed request for mferrera January 25, 2024 13:52
@perolavsvendsen
Copy link
Member

  • Verify Pydantic version with Komodo and RMS environments. We think this should be OK.
  • Existing tests should pass with new schema.
  • Tests should be expanded if coverage is too poor. Scope for this PR??
  • Existing data should validate on the new schema. Scope for this PR??
  • Add more elaborate docstrings on the data classes, for documentation (combine with checking Sphinx capabilities to avoid double work). Later PR, not scope.
  • Check which "volume" variants in the content list that are actually discontinued.
  • Need to be more specific on some of the numerical inputs, e.g. cannot have negative number of layers etc. Later PR, not scope.
  • Build the schema to the Radix app. Later PR, not scope.

Copy link
Member

@perolavsvendsen perolavsvendsen left a comment

Choose a reason for hiding this comment

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

This is only adding functionality - should not have any negative impact on existing usage.

Consider comments.

Very nice start on significant improvement 🚀

@@ -0,0 +1,6 @@
from .model import Root, dump
Copy link
Member

Choose a reason for hiding this comment

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

May cause confusion when the word "model" hits the namespace in a context where this word has lots of meanings. Possible to use "datamodels"?

Copy link
Member

Choose a reason for hiding this comment

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

"datastructure" or "datastructures"? Ref discussion

@janbjorge
Copy link
Contributor Author

From review, rename modeles folder to datastructure

Copy link
Collaborator

@jcrivenaes jcrivenaes left a comment

Choose a reason for hiding this comment

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

🚀

@janbjorge
Copy link
Contributor Author

Followups:

  • It seems that SUMO will add timezone if not given, leads to unnecessary union of aware and naive datetime (se code comments).
  • RealizationJobs is to strict, or sumo is lacking information in old cases. Needs followup.
python3 tools/sumo-explorer-model-validate.py
Traceback (most recent call last):
  File "/Users/JLOV/src/fmu-dataio/tools/sumo-explorer-model-validate.py", line 53, in <module>
    parsed = Root.model_validate(m)
  File "/Users/JLOV/.pyenv/versions/dataio/lib/python3.10/site-packages/pydantic/main.py", line 503, in model_validate
    return cls.__pydantic_validator__.validate_python(
pydantic_core._pydantic_core.ValidationError: 16 validation errors for Root
cube.fmu.realization.jobs.ert_pid
  Field required [type=missing, input_value={'umask': '0002', 'DATA_R...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.global_environment
  Field required [type=missing, input_value={'umask': '0002', 'DATA_R...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.arg_types
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.argList
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.error_file
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.executable
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.license_path
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.max_arg
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.max_running_minutes
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.max_running
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.start_file
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.stderr
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.stdin
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.stdout
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.jobList.0.target_file
  Field required [type=missing, input_value={'min_arg': 1, 'name': 'M...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing
cube.fmu.realization.jobs.run_id
  Field required [type=missing, input_value={'umask': '0002', 'DATA_R...'anything': 'something'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing

@janbjorge janbjorge merged commit e5fa542 into equinor:main Jan 25, 2024
12 checks passed
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.

4 participants