8000 [MRG]Diabetes example with GridSearchCV by rishikksh20 · Pull Request #8268 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG]Diabetes example with GridSearchCV #8268

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 3 commits into from
Feb 7, 2017
Merged

[MRG]Diabetes example with GridSearchCV #8268

merged 3 commits into from
Feb 7, 2017

Conversation

rishikksh20
Copy link
Contributor

Rewrite the diabetes example with GridSearchCV

Reference Issue

Fixes #8248

What does this implement/fix? Explain your changes.

Modify diabetes example for GridSearchCV

Any other comments?

@rishikksh20 rishikksh20 changed the title Diabetes example with GridSearchCV [WIP]Diabetes example with GridSearchCV Feb 2, 2017
@lesteve
Copy link
Member
lesteve commented Feb 2, 2017

Quickly looked at the doc in your first commit and the output of the example does not change so this looks good. I'll wait for CircleCI to come back to have another look.

@lesteve
Copy link
Member
lesteve commented Feb 2, 2017

Also use [MRG] in the title when you think the PR is ready for review.

@rishikksh20 rishikksh20 changed the title [WIP]Diabetes example with GridSearchCV [MRG]Diabetes example with GridSearchCV Feb 2, 2017
tguillemot
tguillemot approved these changes Feb 2, 2017
View reviewed changes
Copy link
Contributor
@tguillemot tguillemot left a comment

Choose a reason for hiding this comment

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

LGTM. Results seems similar.

examples/exercises/plot_cv_diabetes.py Outdated
scores.append(np.mean(this_scores))
scores_std.append(np.std(this_scores))

scores, scores_std = np.array(scores), np.array(scores_std)

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the blank line.

lesteve
lesteve reviewed Feb 2, 2017
View reviewed changes
Copy link
Member
@lesteve lesteve left a comment

Choose a reason for hiding this comment

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

Small comments. I feel the naming was better in the previous version so keep it like it was.

examples/exercises/plot_cv_diabetes.py Outdated

clf = GridSearchCV(lasso, tuned_parameters, cv=n_folds, refit=False)
clf.fit(X, y)
means = clf.cv_results_['mean_test_score']
Copy link
Member

Choose a reason for hiding this comment

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

Can you keep the name scores for this variable?

examples/exercises/plot_cv_diabetes.py Outdated
clf = GridSearchCV(lasso, tuned_parameters, cv=n_folds, refit=False)
clf.fit(X, y)
means = clf.cv_results_['mean_test_score']
stds = clf.cv_results_['std_test_score']
Copy link
Member

Choose a reason for hiding this comment

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

Can you keep the name scores_std for this variable?

@rishikksh20
Diabetes example with GridSearchCV
6c63b81 
Rewrite the diabetes example with GridSearchCV

Issue:#8248
@rishikksh20
Modify diabetes example to remove pep8 error
0c72cbc 
Modify a code to remove pep8 error while building

Ressolve issue: #8248
@rishikksh20
Cosmetic Changes in Diabetes Example as discussed
b4f298d 
Remove extra blank line and change variable names

Issue: #8248
@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

Please use another branch for your codecov work.

If you revert your last commit, I actually think that your PR was mergeable.

@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

Please use another branch for your codecov work.

Also use meaningful branch names as far as possible.

@rishikksh20
Copy link
Contributor Author

@lesteve All further commits are reverted.

@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

@lesteve All further commits are reverted.

I'll merge this when the CI come back green. @rishikksh20 please ping me if I forget.

@rishikksh20
Copy link
Contributor Author

@lesteve please merge it .

@lesteve lesteve merged commit 3c6f2b0 into scikit-learn:master Feb 7, 2017
@lesteve
Copy link
Member
lesteve commented Feb 7, 2017

Merged, thanks a lot

@rishikksh20 rishikksh20 deleted the rishi_patch branch February 17, 2017 07:32
sergeyf pushed a commit to sergeyf/scikit-learn that referenced this pull request Feb 28, 2017
@rishikksh20 @sergeyf
[MRG] Diabetes example with GridSearchCV (scikit-learn#8268)
69a4a59
@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
Closed
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
@rishikksh20 @Sundrique
[MRG] Diabetes example with GridSearchCV (scikit-learn#8268)
887a100
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
@rishikksh20 @NelleV
[MRG] Diabetes example with GridSearchCV (scikit-learn#8268)
fa9e23a
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
@rishikksh20 @paulha
[MRG] Diabetes example with GridSearchCV (scikit-learn#8268)
74f3ad5
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
@rishikksh20 @maskani-moh
[MRG] Diabetes example with GridSearchCV (scikit-learn#8268)
0598a53
lemonlaug pushed a commit to lemonlaug/scikit-learn that referenced this pull request Jan 6, 2021
@rishikksh20
[MRG] Diabetes example with GridSearchCV (scikit-learn#8268)
cea39bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesteve lesteve lesteve left review comments

@tguillemot tguillemot tguillemot approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rewrite diabetes example with GridSearchCV?
3 participants
@rishikksh20 @lesteve @tguillemot
0