8000 DOC Update notebook style for plot_bayesian_ridge_curvefit by 2357juan · Pull Request #22916 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC Update notebook style for plot_bayesian_ridge_curvefit #22916

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 3 commits into from
Mar 30, 2022

Conversation

2357juan
Copy link
Contributor

DOC Update notebook style for plot_bayesian_ridge_curvefit

Reference Issues/PRs

Update examples/linear_model/plot_bayesian_ridge_curvefit.py to notebook style, Issue #22406

What does this implement/fix? Explain your changes.

Split into 3 sections

  • Data generation
  • Fitting
  • Plotting

Moved the following code

  • Sinusoidal data generating function to the data generation code block
  • BayesianRidge import to fitting code block

@jeremiedbb jeremiedbb added the Quick Review For PRs that are quick to review label Mar 22, 2022
@@ -28,33 +28,36 @@

# Author: Yoshihiro Uchida <nimbus1after2a1sun7shower@gmail.com>

# %%
# Generate sinusoidal data with noise
# -----------------------------------
import numpy as np
import matplotlib.pyplot as plt
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest moving this import down to the plotting code block where it is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 59 to 61
from sklearn.linear_model import BayesianRidge

reg = BayesianRidge(tol=1e-6, fit_intercept=False, compute_score=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on your PR comment, I suspect you meant to move these lines into the previous fitting block rather than this plotting one. I agree that the fitting block would be a better place for these lines given that this final block is only concerned with plotting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Thanks for taking the time to review!

@lesteve lesteve mentioned this pull request Mar 28, 2022
47 tasks
@lesteve
Copy link
Member
lesteve commented Mar 30, 2022

LGTM, merging, thanks a lot!

@lesteve lesteve merged commit 24ef388 into scikit-learn:main Mar 30, 2022
@lesteve
Copy link
Member
lesteve commented Mar 30, 2022

And thanks @jsilke for the helpful comments!

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4BE4 Documentation Quick Review For PRs that are quick to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0