-
Notifications
You must be signed in to change notification settings - Fork 13
feat: Support Dataframes as Fields in Pydantic Models #148
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
base: main
Are you sure you want to change the base?
Conversation
|
||
@classmethod | ||
@abstractmethod | ||
def validate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to move this here as an abstract class method because the generic argument to dy.Dataframe
is only bound to BaseSchema
, but I need to be able to call validate
on it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 50 51 +1
Lines 2851 2906 +55
=========================================
+ Hits 2851 2906 +55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pixi.toml
Outdated
pytest-cov = "*" | ||
pytest-md = "*" | ||
scikit-learn = "*" | ||
pydantic = ">=2.11.9,<3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure: Since we also use the Pydantic mypy plugin, does Pydantic have to be added to the docs
feature? I think no, but I haven't checked exactly how you generate the docs for dataframely.mypy
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @ytausch!
I have no pre-existing experience with custom types in pydantic, so my review on that front relies on understanding the tests. Overall, I just have pretty minor comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side note: there are some CI checks not marked as required, is that intended? |
Nope, fixed, thanks for pointing it out :) |
Co-authored-by: Andreas Albert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, thanks for bearing with all the fine-tuning :) I just checked in with @borchero and he asked to take a look before we merge, so let's please wait for his review.
Closes #143.
If this is released, the feedstock should add a version constraint on
pydantic_core
.