search

rom-py / rompy

Relocatable Ocean Modelling in PYthon (rompy) combines templated cookie-cutter model configuration with various xarray extensions to assist in the setup and evaluation of coastal ocean model
https://rom-py.github.io/rompy/
BSD 3-Clause "New" or "Revised" License
2 stars 9 forks source link

Serializable slices #52

Closed benjaminleighton closed 7 months ago

benjaminleighton commented 8 months ago

Filter objects used in DataGrid.filter have a dictionary Filter.crop that typically consists of variable names as keys and slice objects as values. In Filter.crop is used when Filter.crop_filter is applied on call. At this point data is cropped according to specified slices. Python slice objects are not serializable in Pydantic. Thus I've

Create a serializable class Slice in types. This type can be converted to and from slice objects using the included method and support slicing by float, str or datetime. Furthermore it supports being deserialized from slice objects for programmatic cases where slices are passed as key value args in a dictionary to an object that creates a hierarchy with a Slice object in it. Additional Filter objects have a validator that will convert slice objects in the crop dictionary supporting legacy slice objects in tests but also allowing this as a valid use.

Additionally this pull request contains a minor bug fix to groups allowing some properties to be None on deserialization from json or yaml.

rafa-guedes commented 7 months ago

@benjaminleighton sorry for taking long to review this. It all looks good, I'm merging it. I just made a couple of minor changes to (1) replace the deprecated pydantic validator by a field_validator, and (2) set default values in the new Slice type using Field to help with auto documentation. Just out of curiosity, do you have an example where those properties of the OUTPUT group object would be None please? Those properties should be defined by the fields explicitly specified only so just wondering in which cases those would take None values.