Skip to content

JSON Schema oil type improvements#59

Merged
honzaflash merged 7 commits into
mainfrom
schema-improvements
Nov 19, 2025
Merged

JSON Schema oil type improvements#59
honzaflash merged 7 commits into
mainfrom
schema-improvements

Conversation

@honzaflash

Copy link
Copy Markdown
Contributor

What

Mainly refactors related to the oil type:

  • split up the enums file to a few smaller ones
  • change the OilTypeEnum values (and names) to be just IDs
  • add the oil type labels to the JSON schema without an extra override
  • update the code to work with the simplified oil type
  • update the docs (tutorial.md) to reflect this
  • a little tangential refactor of the _update_od_config_from_this_config body to reduce indentation

Why

To have oil type enum-label pairs in the JSON schema and make oil type parameter easier to use.

All the tests passed and if I remember correctly, I successfully ran a simulation after the last commit too.

Note: once #54 is merged, I'll change the PR target to main (this branch is already based on the kerchunk fixes).

- create an `enums` subpackage that exports everything `enums.py` module did
- original contentes of `enums.py` get split into multiple smaller files though
- make it's value a simple string (matching the name) - no more tuple
- get rid of the pydantic field "before" validator (used to map from id to a tuple)
- adapt usage to the new enum value
- also add a `oneOf` list to `json_schema_extra` so that it's part of the schema by default
- remove the (now redundant) `ModifyOilTypeJsonSchema`
- fix a typo so that Pydantic actually validates default values
- update tests
- `create_ocean_model_simulation` returns a model not model instance
- export the module with opendrift models from the package root
- this way they can be easily imported from PTM when used as a library
- a little tangential refactor
- use "early return" style (or rater `continue`) to reduce indentation in `_update_od_config_from_this_config`
- also iterate over key, val
- also use `x.y` instead of `getattr(x, y)` where possible
@honzaflash
honzaflash changed the base branch from fix-kerchunk-files to main November 19, 2025 19:09
@honzaflash honzaflash changed the title Draft: JSON Schema oil type improvements JSON Schema oil type improvements Nov 19, 2025
@kthyng

kthyng commented Nov 19, 2025

Copy link
Copy Markdown
Collaborator

Ok I've checked a bunch of stuff and it looks good but I have a question: what should a user do if they know the name of the oil type they want to use but not the id?

@honzaflash

Copy link
Copy Markdown
Contributor Author

what should a user do if they know the name of the oil type they want to use but not the id?

Well, previously they would need to provide the ID or both ID and name, right? yes. So not much changes there.

However, still not a bad question - I think they'd have to look it up. Either, online through the database or using the library's models or enums.

That made me check and I think I should update this line in the docs:

ptm.OpenOilModelConfig.model_json_schema()["$defs"]["OilTypeEnum"]["enum"]

That should still work but now it will only have the IDs not the tuple. Alternatively, we could point them to

  1. the schema's oneOf list of {const: ..., title: ...} objects or
  2. the enum class itself at ptm.opendrift_models.enums.OilTypeEnum and say something like you can do {oil.value: oil.title for oil in OilTypeEnum} to get id to name map.

@kthyng

kthyng commented Nov 19, 2025

Copy link
Copy Markdown
Collaborator

I don’t think PTM needs to accept oil type by name but I do think it would be good to explain how to get it. Two ways:

  1. The ADIOS database: https://adios.orr.noaa.gov/oils
  2. or from the code. There are duplicate names in OIL_ID_TO_NAME so that has to be accommodated in any suggestion we give. I went ahead and added a helper dict so that a user could use ptm.models.opendrift.enums.oil_types.NAME_TO_OIL_ID to find the id or ids they need to choose from for input to the model. This could be added to the docs along with a link to the ADIOS database if not already there. As an example: ptm.models.opendrift.enums.oil_types.NAME_TO_OIL_ID["ALASKA NORTH SLOPE"] returns two ids (and note you get a different answer if the name is not all caps!).

What do you think about this?

- add a dict for name to ID mapping
- add notes to the `docs/tutorial.md` about the oil types
@honzaflash
honzaflash merged commit 2b553f3 into main Nov 19, 2025
5 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.

2 participants