10000 ENH Support sample weights in PartialDependenceDisplay.from_estimator by vitaliset · Pull Request #26644 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

ENH Support sample weights in PartialDependenceDisplay.from_estimator #26644

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 22, 2023
Merged

ENH Support sample weights in PartialDependenceDisplay.from_estimator #26644

merged 5 commits into from
Jun 22, 2023

Conversation

vitaliset
Copy link
Contributor

This PR fixes #24872.
Built on top of #25209.

As partial dependence of a model at a point is defined as an expectation, as #24872 points out, it should respect sample_weight if someone wishes to use it (for instance, when you know your X does not follow the distribution you are interested in).

The prior PR (#25209) incorporated this logic into the partial_dependence_plot function. This current PR completes the integration by incorporating it into PartialDependenceDisplay.from_estimator.

@github-actions
Copy link
github-actions bot commented Jun 21, 2023 •
edited
Loading

✔️ Linting Passed

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

Generated for commit: 956002d

@vitaliset
Copy link
Contributor Author

Kindly pinging @glemaitre, @lorentzenchr, @jeremiedbb here as you might be interested and already familiar with the previous PR.
Maybe we can add a 1.3 milestone to attract reviewers. This PR should be easy to review and easy for me to edit. ;)

I wasn't creative in the test, as it was unclear to me what we want to evaluate. Therefore, any suggestions are welcome!

glemaitre
glemaitre approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member
@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

It looks good to me.
@jeremiedbb I think we need this one not to have a discrepancy between partial_dependence and the display in 1.3.

@jeremiedbb jeremiedbb added this to the 1.3 milestone Jun 21, 2023
@jeremiedbb jeremiedbb added the To backport PR merged in master that need a backport to a release branch defined based on the milestone. label Jun 21, 2023
@jeremiedbb
Copy link
Member

I'm okay with that

thomasjpfan
thomasjpfan approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@vitaliset
Copy link
Contributor Author

I just updated the branch with main. CI is green. I'm ok with it being merged as is. :)

@jeremiedbb
Copy link
Member

Thanks @vitaliset

@jeremiedbb jeremiedbb merged commit 5e8d8cb into scikit-learn:main Jun 22, 2023
@vitaliset vitaliset deleted the pdp_plot_sw branch June 22, 2023 15:36
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jun 29, 2023
@vitaliset @jeremiedbb
ENH Support sample weights in PartialDependenceDisplay.from_estimator (…
4f9f629 
…scikit-learn#26644)
jeremiedbb pushed a commit that referenced this pull request Jun 29, 2023
@vitaliset @jeremiedbb
ENH Support sample weights in PartialDependenceDisplay.from_estimator (…
25e179a 
…#26644)
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@vitaliset @REDVM
ENH Support sample weights in PartialDependenceDisplay.from_estimator (…
b51c96c 
…scikit-learn#26644)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan thomasjpfan approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
module:inspection To backport PR merged in master that need a backport to a release branch defined based on the milestone.
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

partial_dependence should respect sample weights
4 participants
@vitaliset @jeremiedbb @thomasjpfan @glemaitre
0