Skip to content

feat: take pipeline's estimator into account to decide tabular vectorizer options in TabularPipeline - #2152

Merged
lisaleemcb merged 17 commits into
skrub-data:mainfrom
khaoulariad:feature_add_skpipeline_to_tabular_pipeline
Sep 15, 2026
Merged

lisaleemcb merged 17 commits into
skrub-data:mainfrom
khaoulariad:feature_add_skpipeline_to_tabular_pipeline

Conversation

@khaoulariad

@khaoulariad khaoulariad commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

Bug Fix Pull Request

Description

Addresses #1967

Checklist

  • I have read the contributing guidelines
  • I have added tests that verify the bug fix
  • I have added an entry to CHANGES.rst describing the fix
  • My code follows the code style of this project
  • I have checked my code and corrected any misspellings

How Has This Been Tested?

AI Disclosure

  • This PR contains AI-generated code
    • I have tested the code generated in my PR
    • I have read and understood every line that has been generated by the AI agent
    • I can explain what the AI-generated code does

Comment thread skrub/tests/test_tabular_pipeline.py Outdated
Comment thread skrub/_tabular_pipeline.py Outdated
@MarieSacksick MarieSacksick added the CFM sprint June 2026 For PRs opened during the CFM sprint in June 2026 label Jun 10, 2026
Comment thread CHANGES.rst Outdated
Comment thread skrub/_tabular_pipeline.py Outdated
Comment thread skrub/tests/test_tabular_pipeline.py Outdated
Comment thread skrub/tests/test_tabular_pipeline.py Outdated
Comment thread skrub/tests/test_tabular_pipeline.py
@MarieSacksick

Copy link
Copy Markdown
Contributor

Can you add the link to this issue that will be closed by this PR please?

Comment thread skrub/_tabular_pipeline.py Outdated
correcting doc

Co-authored-by: Marie Sacksick <79304610+MarieSacksick@users.noreply.github.com>
Comment thread CHANGES.rst Outdated
Co-authored-by: Marie Sacksick <79304610+MarieSacksick@users.noreply.github.com>
Comment thread skrub/tests/test_tabular_pipeline.py Outdated
@MarieSacksick MarieSacksick changed the title add skpipeline to tabular pipeline feat: take estimator in pipeline when given to TabularPipeline into account to decide tabular vectorizer options Jun 10, 2026
@MarieSacksick MarieSacksick changed the title feat: take estimator in pipeline when given to TabularPipeline into account to decide tabular vectorizer options feat: take pipeline's estimator into account to decide tabular vectorizer options in TabularPipeline Jun 10, 2026
Co-authored-by: Marie Sacksick <79304610+MarieSacksick@users.noreply.github.com>

@MarieSacksick MarieSacksick left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good for me, thanks :)!
and congratulations!!

waiting for someone else review.

@jeromedockes jeromedockes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thank you very much @khaoulariad ! just a few comments :)

Comment thread skrub/_tabular_pipeline.py Outdated
Comment thread skrub/_tabular_pipeline.py Outdated
Comment thread skrub/_tabular_pipeline.py Outdated
Comment thread skrub/tests/test_tabular_pipeline.py
Comment thread skrub/tests/test_tabular_pipeline.py Outdated
Comment thread skrub/tests/test_tabular_pipeline.py Outdated
Comment thread skrub/tests/test_tabular_pipeline.py Outdated
@rcap107

rcap107 commented Aug 18, 2026

Copy link
Copy Markdown
Member

Hi @khaoulariad, this PR hasn't seen any action in a long time. Are you still interested in working on this?

@lisaleemcb lisaleemcb self-assigned this Aug 26, 2026
@rcap107 rcap107 modified the milestones: Release 0.10.1, Release 0.11 Aug 28, 2026

@rcap107 rcap107 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PR is almost done 🎉 , the conflict is caused by the documentation move.

The entry about this PR in the changelog is in the wrong location: it should be moved to the ongoing development section.

After that's done, we can merge.

@lisaleemcb
lisaleemcb dismissed GaelVaroquaux’s stale review September 15, 2026 13:18

This is updated already

@lisaleemcb
lisaleemcb merged commit 5474d02 into skrub-data:main Sep 15, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CFM sprint June 2026 For PRs opened during the CFM sprint in June 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants