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

Ideas from Pydantic #580

Open
MarcSkovMadsen opened this issue Dec 11, 2021 · 2 comments
Open

Ideas from Pydantic #580

MarcSkovMadsen opened this issue Dec 11, 2021 · 2 comments
Labels
type-feature Feature request
Milestone

Comments

@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Dec 11, 2021

I'm currently exploring integration between Param and Pydantic.

I will use this issue to highlight some things Param could learn or otherwise utilize from Pydantic.

1. Pydantic is compiled with Cython to make it super fast.

Could/ should Param be?

2. Pydantic has a very fast and flexible deserialization

Could Param inherit that code just as Pydantic inherited from Django? For example for int it will deserialize both 123 and "123" to 123. For example for datetimes it can deserialize a wide range of formats.

For example for parsing datetime https://github.com/samuelcolvin/pydantic/blob/master/pydantic/datetime_parse.py.

@MarcSkovMadsen MarcSkovMadsen added type-feature Feature request TRIAGE User-submitted issue that hasn't been triaged yet. labels Dec 11, 2021
@MarcSkovMadsen MarcSkovMadsen changed the title Learnings from Pydantic Ideas from Pydantic Dec 11, 2021
@jbednar
Copy link
Member

jbednar commented Dec 15, 2021

  1. Pydantic is compiled with Cython to make it super fast.

I don't know if anyone is using it or if it's still supported, but at least at some point Param could be compiled with Cython (#122). Happy for someone to investigate that and add it to the docs if it's still viable!

Personally, I think of Param as focused on the human-scale usage of an API, not on the internal communication inside deeply nested loops, so Param would not normally be a bottleneck. But if someone identifies bottlenecks in real code and wants to optimize for them, I'm happy for that to happen as long as Param stays simple and pure Python by default, so that it is safe to include in any project without increasing its build complexity or dependencies.

  1. Pydantic has a very fast and flexible deserialization

It would definitely be useful to implement some forgiving, flexible conversions alongside the usual strict ones. As discussed in #578 (comment) , we have to be careful not to change the semantics of Param, which inherently focuses on giving feedback to the user about whether they supplied something invalid (e.g. when configuring a large batch job by hand coding parameter values). But it's certainly useful to have a separate entry point, along the lines of set_in_bounds(), that is more permissive and focuses on coercion rather than raising exceptions.

For the specific issue of datetimes, extending the constructor and setattr to support a provided string seems reasonable, given how awkward it is to create something as a datetime manually. I don't think doing so would threaten the overall focus of Param on giving validation to the user.

@maximlt
Copy link
Member

maximlt commented Dec 16, 2021

For the specific issue of datetimes, extending the constructor and setattr to support a provided string seems reasonable, given how awkward it is to create something as a datetime manually. I don't think doing so would threaten the overall focus of Param on giving validation to the user.

Is it really that awkward? I never thought so. This reads pretty easily to me: datetime.datetime(2018, 6, 1, 13, 45, 0).

@MridulS MridulS added this to the Wishlist milestone Feb 7, 2022
@MridulS MridulS removed the TRIAGE User-submitted issue that hasn't been triaged yet. label Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature Feature request
Projects
None yet
Development

No branches or pull requests

4 participants