Skip to content

Conversation

@satvshr
Copy link
Contributor

@satvshr satvshr commented Jan 22, 2026

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:

  1. test_get_data_pandas bug: Solved type error for dataframe columns having str datatype for pandas==3 and object for older versions.
  2. test_get_sparse_dataset_dataframe, test_get_sparse_dataset_rowid_and_ignore_and_target, and test_get_sparse_dataset_dataframe_with_target bug: typecasting type_ to a np array.
  3. bugs in test_flow_functions.py: ext_version can now be nan because of pandas 3.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.75%. Comparing base (cf8e9db) to head (8010a20).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@geetu040 geetu040 left a 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!

Comment on lines +105 to +107
# Dynamically detect what this version of Pandas calls string columns.
str_dtype = data["name"].dtype.name

Copy link
Collaborator

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?

Copy link
Contributor Author

@satvshr satvshr Jan 23, 2026

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.

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.

[BUG] Test failures

3 participants