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

FIX warn when requested polars DataFrame but no column names are found #28263

Merged
merged 5 commits into from Jan 30, 2024

Conversation

glemaitre
Copy link
Member

While working on #28260, I found out that we are not warning about not returning a polars DataFrame or requesting column names as we do for pandas in the FunctionTransformer.

This is particularly disturbing because you get an array but without really knowing the reason.

Copy link

github-actions bot commented Jan 25, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 6bda3ee. Link to the linter CI: here

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. Or did you want to change sth here?

@adrinjalali adrinjalali added this to the 1.4.1 milestone Jan 25, 2024
@glemaitre
Copy link
Member Author

Or did you want to change sth here?

I just wanted to make explicit the type of DataFrame. This is done with a stronger match in the error to be sure that we are explicit.

@lesteve lesteve enabled auto-merge (squash) January 30, 2024 16:36
@lesteve
Copy link
Member

lesteve commented Jan 30, 2024

I fixed the conflict in the changelog and enabled auto-merge

@lesteve lesteve merged commit d797b77 into scikit-learn:main Jan 30, 2024
25 checks passed
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 10, 2024
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 13, 2024
glemaitre added a commit that referenced this pull request Feb 13, 2024
#28263)

Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants