Skip to content
This repository was archived by the owner on Jun 3, 2025. It is now read-only.

Conversation

@bfineran
Copy link
Contributor

  • bumped the minimum supported pydantic version to 1.5.0 for support of default_factory

@bfineran bfineran requested review from a team and markurtz October 12, 2021 21:26
@bfineran bfineran self-assigned this Oct 12, 2021
@bfineran bfineran requested review from horheynm and mgoin and removed request for a team October 12, 2021 21:26
rahul-tuli
rahul-tuli previously approved these changes Oct 12, 2021
Copy link
Member

@rahul-tuli rahul-tuli left a comment

Choose a reason for hiding this comment

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

LGTM!

markurtz
markurtz previously approved these changes Oct 14, 2021
Copy link
Member

@markurtz markurtz left a comment

Choose a reason for hiding this comment

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

Looks great Ben, I also love how much pydantic takes over for us to remove the boilerplate! Once this lands, it might be good to cherry pick this over for 0.8

@bfineran bfineran dismissed stale reviews from markurtz and rahul-tuli via 97d720b October 14, 2021 15:24
@bfineran
Copy link
Contributor Author

thanks @markurtz, your point actually made me realize that pydantic can handle the JSON serialization for the layer info and result objects, so I've updated this. the only drawback is that pydantic includes optionals that are None in their serialized dicts.

@rahul-tuli
Copy link
Member

@bfineran bfineran merged commit eb67ee6 into main Oct 14, 2021
@bfineran bfineran deleted the model-info-pydantic branch October 14, 2021 17:34
@bfineran bfineran restored the model-info-pydantic branch October 14, 2021 21:40
bfineran added a commit that referenced this pull request Oct 14, 2021
* migrate LayerInfo and analysis result classes to pydantic

* bump pydantic min version to 1.5.0 (default_factory support)

* removing boilerplate JSON serialization fns for results and layer info in favor of pydantic built-ins
@bfineran bfineran deleted the model-info-pydantic branch October 14, 2021 21:41
markurtz pushed a commit that referenced this pull request Oct 19, 2021
… (#411)

* migrate LayerInfo and analysis result classes to pydantic

* bump pydantic min version to 1.5.0 (default_factory support)

* removing boilerplate JSON serialization fns for results and layer info in favor of pydantic built-ins
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants