search

JakobGM / patito

A data modelling layer built on top of polars and pydantic
MIT License
321 stars 25 forks source link

feat: Allow clients to pass json_schema_extra too #88

Closed adrienDog closed 2 months ago

adrienDog commented 3 months ago

Currently, if a client passes json_schema_extra they would get the following error:

TypeError: pydantic.fields.Field() got multiple values for keyword argument 'json_schema_extra'

Instead, we should merge the two json_schema_extra dicts if one is passed form the client

thomasaarholt commented 3 months ago

Thank you! Could you add a test as well, adding something that a user might add to json_schema_extra, just to check that nothing breaks when the user does so?

adrien-owkin commented 3 months ago

Thank you! Could you add a test as well, adding something that a user might add to json_schema_extra, just to check that nothing breaks when the user does so?

Hi Thomas! thank you for the quick review, yes for sure I will add the test sorry for not doing it in the first place :)

On my side this is needed as a workaround so I can add ColumnMetadata to our fields. We indeed have the need to add for example {'category': 'a', 'source': 'https://wwwww'} etc... I see patito uses the json_schema_extra for a similar purpose since it seems to be the only prop we can leverage for this.

Do you think later on I could suggest an additional prop to patito ColumnInfo? sthg like metadata: Dict it is not really supported by polars https://github.com/pola-rs/polars/issues/5117 but we could still add it to Patito maybe?

adrienDog commented 3 months ago

@thomasaarholt i added a test, thank you for the great repo, it was very easy to create the poetry venv, work in VSCode and contribute 🙏

once this is merged, what is the release process assuming this is a patch upgrade? so essentially we would go from 0.7.0 to 0.7.1

adrienDog commented 3 months ago

@thomasaarholt @dsgibbons I rebased and fixed conflicts :)

thomasaarholt commented 2 months ago

This looks like good work to me. Sorry for taking so long at this! I'm aiming to make a new release this weekend, so this will be out on pypi then.

Thanks for the work!

dsgibbons commented 2 months ago

This looks like good work to me. Sorry for taking so long at this! I'm aiming to make a new release this weekend, so this will be out on pypi then.

Do you know when the next release will be @thomasaarholt?