Skip to content

ENH Add support for 'fit_params' to learning_curve - #18595

Merged
thomasjpfan merged 5 commits into
scikit-learn:masterfrom
amy12xx:learning_curve_fit_params
Oct 14, 2020
Merged

thomasjpfan merged 5 commits into
scikit-learn:masterfrom
amy12xx:learning_curve_fit_params

Conversation

@amy12xx

@amy12xx amy12xx commented Oct 11, 2020 •

Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Fixes #10252

What does this implement/fix? Explain your changes.

Add fit_params support to learning_curve

Any other comments?

@amy12xx amy12xx changed the title learning curve fit params support [DRAFT] Add support for 'fit_params' in learning_curve Oct 11, 2020
@amy12xx amy12xx changed the title [DRAFT] Add support for 'fit_params' in learning_curve [DRAFT] Add support for 'fit_params' to learning_curve Oct 11, 2020
@amy12xx
amy12xx marked this pull request as ready for review October 11, 2020 07:37
@amy12xx amy12xx changed the title [DRAFT] Add support for 'fit_params' to learning_curve [MRG] Add support for 'fit_params' to learning_curve Oct 11, 2020

@ogrisel ogrisel 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.

LGTM. Thank you very much for the follow-up @amy12xx. Can you add an entry to the doc/whats_new/0.24.rst file?

Comment thread sklearn/model_selection/_validation.py
def __init__(self, n_max_train_sizes, expected_fit_params=None):
super().__init__(n_max_train_sizes)
self.x = None
self.expected_fit_params = expected_fit_params

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.

@thomasjpfan I believe this is what you requested in #18527. Looks good to me.

@amy12xx

amy12xx commented Oct 14, 2020 •

Copy link
Copy Markdown
Contributor Author

@ogrisel

ogrisel commented Oct 14, 2020

Copy link
Copy Markdown
Member

This was fixed in master by #18617. I think you need to merge master in that branch again.

@ogrisel

ogrisel commented Oct 14, 2020

Copy link
Copy Markdown
Member

I just pushed a merge of master to your branch. Let's see...

@thomasjpfan thomasjpfan 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.

LGTM

@thomasjpfan thomasjpfan changed the title [MRG] Add support for 'fit_params' to learning_curve ENH Add support for 'fit_params' to learning_curve Oct 14, 2020
@thomasjpfan
thomasjpfan merged commit 1e386a4 into scikit-learn:master Oct 14, 2020
amrcode pushed a commit to amrcode/scikit-learn that referenced this pull request Oct 19, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
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.

validation_curve and learning_curve should support fit_params

3 participants