-
-
Notifications
You must be signed in to change notification settings - Fork 213
[BUG] Test Failures caused because of pandas 3 #1628
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1628 +/- ##
==========================================
+ Coverage 52.36% 52.75% +0.39%
==========================================
Files 36 36
Lines 4333 4333
==========================================
+ Hits 2269 2286 +17
+ Misses 2064 2047 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
geetu040
left a comment
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.
Really nice, this fixes the bugs. Just left a comment for discussion, otherwise LGTM!
| # Dynamically detect what this version of Pandas calls string columns. | ||
| str_dtype = data["name"].dtype.name | ||
|
|
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.
Isn't that cheating? why would it ever fail for "name" column if we are dynamically extracting the dtype. Is it to make sure it works with pandas<3.0.0?
would make sense to replace "object" simply with "str" in col_dtype if we are setting pandas>=3.0.0 in dependencies
CC: @fkiraly, are we bounding to this pandas version in pyproject.toml?
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.
Isn't that cheating?
It is definitely cheating! Thought it was better to get a quick fix out and then fine-tune the fixes later on. It was between this or bounding pandas, and for me this was the better/less intense option between the 2.
Metadata
Reference Issue: fixes [BUG] Test failures #1627
What does this PR implement/fix? Explain your changes.
This PR fixes the 7 recurring bugs across all current PRs because of pandas 3:
test_get_data_pandasbug: Solved type error for dataframe columns havingstrdatatype forpandas==3andobjectfor older versions.test_get_sparse_dataset_dataframe,test_get_sparse_dataset_rowid_and_ignore_and_target, andtest_get_sparse_dataset_dataframe_with_targetbug: typecastingtype_to a np array.test_flow_functions.py:ext_versioncan now benanbecause ofpandas 3.