Skip to content

Fix merge issues for better SVR plot #12877

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

Merged
merged 4 commits into from
Jan 9, 2019
Merged

Conversation

mferrari3
Copy link
Contributor

@mferrari3 mferrari3 commented Dec 27, 2018

Reference Issues/PRs
This fixes #8365
This also closes #8367 and closes #12207

What does this implement/fix? Explain your changes.
This fixes the merge errors because the only failure was with the python 2.7 test and the update master no longer tests in python 2.7.

Any other comments?
cc: @reshamas
#wimlds

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

Thanks

plt.legend()
fig, (ax1, ax2, ax3) = plt.subplots(1, 3, figsize=(15, 10), sharey=True)

ax1.plot(X, y_rbf, color='m', lw=lw, label='RBF model')
Copy link
Member

Choose a reason for hiding this comment

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

Could we please make use of a loop to avoid this repeated code now that we have three separate plots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jnothman. I created a loop for the plots, committed the code, and passed testing. Please let me know if there is anything else.

plt.title('Support Vector Regression')
plt.legend()

svrs = [svr_rbf, svr_lin, svr_poly]
Copy link
Member

Choose a reason for hiding this comment

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

I would perhaps rather an idiom with models = [{'est': svr_rbf, 'label': 'RBF', 'color': 'm', 'ax': axes[0]}], but this is fine by me.

@jnothman
Copy link
Member

jnothman commented Jan 9, 2019

Thanks!

@jnothman jnothman merged commit 354c8c3 into scikit-learn:master Jan 9, 2019
@reshamas
Copy link
Member

reshamas commented Jan 9, 2019

thank you!! @mferrari3 @jnothman and also to @ml4713 & @vasudhathinks

jnothman pushed a commit to jnothman/scikit-learn that referenced this pull request Feb 19, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SVR example misleading
3 participants