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

Bugfix/763: DataFrameModel validate returns the correct type #1450

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chadwhawkins
Copy link

Updates the return type casting of various model methods to return the correct type (DataFrame instead of DataFrameBase). The issue called for typing overload but that didn't seem to be necessary in looking at the code, because as best as I can tell calling these methods should always return the same thing (and DataFrame inherits from DataFrameBase anyway).

Resolves #763

Signed-off-by: Chad Hawkins <chawkins@indigoag.com>
Signed-off-by: Chad Hawkins <chawkins@indigoag.com>
Signed-off-by: Chad Hawkins <chawkins@indigoag.com>
Signed-off-by: Chad Hawkins <chawkins@indigoag.com>
@cosmicBboy
Copy link
Collaborator

Hi @chadwhawkins, so unfortunately this will introduce another issue, which is that these methods can also return modin, pyspark.pandas, and dask dataframes. The current typing is also incorrect, but we actually do need to do type overloading here to cover those other dataframe types.

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (812b2a8) 94.28% compared to head (08efe7a) 94.28%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1450      +/-   ##
==========================================
- Coverage   94.28%   94.28%   -0.01%     
==========================================
  Files          91       91              
  Lines        7013     7011       -2     
==========================================
- Hits         6612     6610       -2     
  Misses        401      401              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chadwhawkins
Copy link
Author

Ah ok, thanks @cosmicBboy. I'll continue iterating on this PR.

@cosmicBboy
Copy link
Collaborator

Thanks @chadwhawkins let me know when you need a review on this

@cosmicBboy
Copy link
Collaborator

@chadwhawkins you can rebase on main to address the linter errors: #1468

@chadwhawkins
Copy link
Author

Thanks @cosmicBboy, planning to revisit this soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mypy - SchemaModel.validate does not return a DataFrame
2 participants