-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
TST Fix openml parser implementation for pandas-dev #26386
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
Conversation
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.
This looks better to me. Does it need a changelog? It might be changing dtypes in some cases probably?
I would consider this a fix because we have better dtypes. We should have an entry in what's new to acknowledge it. |
@@ -925,7 +925,7 @@ def datasets_missing_values(): | |||
# with casting it will be transformed to either float or Int64 | |||
(40966, "pandas", 1, 77, 0), | |||
# titanic | |||
(40945, "liac-arff", 3, 5, 0), |
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 am a bit confused by this PR: why would fixing the pandas
parser impact a test for the liac-arff
parser?
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.
This PR is fixing the liac-arff
parser which uses pandas when as_frame=True
.
For this specific tests, the "body" feature is interpreted as a object dtype on main. With this PR, the "body" feature is a float dtype, which increases the expected_n_floats
by one.
Merging, thanks a lot! |
Reference Issues/PRs
Partial Addresses #26154
Alternative to #26344
What does this implement/fix? Explain your changes.
This PR uses the second chunk to infer the dtypes and uses those types for the first chunk. For the titanic dataset, this means that
body
feature gets correctly inferred as a numeric dtype. OpenML denotes body as a numeric feature in it's metadata.