The Wayback Machine - https://web.archive.org/web/20220227183540/https://github.com/scikit-learn/scikit-learn/pull/22461
Skip to content
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

Jump to bottom

DOC: use notebook-style for plot_stock_market.py #22461

Merged
merged 11 commits into from Feb 25, 2022
Merged

DOC: use notebook-style for plot_stock_market.py #22461

merged 11 commits into from Feb 25, 2022

Conversation

@aditya172926
Copy link
Contributor

@aditya172926 aditya172926 commented Feb 12, 2022 •
edited

Reference Issues/PRs

Updates examples/applications/plot_stock_market.py
For Issue Fix notebook-style examples #22406

What does this implement/fix? Explain your changes.

Updated the example plot_stock_market.py to notebook style. Also fixed minor wordings as seemed fit.

Any other comments?
@aditya172926 aditya172926 changed the title fix notebook-style for plot_stock_market DOC FIX notebook-style for plot_stock_market Feb 13, 2022
@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Feb 14, 2022

For this example, it would be as well nice to move the section and discussion of the top summary next to the cell of code that implements what is written in the discussion.

In this way, the example will really feel like a real notebook and will be easier to read.

@aditya172926
Copy link
Contributor Author

@aditya172926 aditya172926 commented Feb 15, 2022

I have done the changes as you said @glemaitre . Have a look.

glemaitre
glemaitre reviewed Feb 15, 2022
View changes
Copy link
Contributor

@glemaitre glemaitre left a comment

This is improving the example greatly. Only a couple of things to address. I will check a bit more in detail why the GraphicalLassoCV is raising a warning.

examples/applications/plot_stock_market.py Outdated Show resolved Hide resolved
examples/applications/plot_stock_market.py Show resolved Hide resolved
examples/applications/plot_stock_market.py Outdated Show resolved Hide resolved
@lesteve lesteve mentioned this pull request Feb 15, 2022
Open
47 tasks
@aditya172926
Copy link
Contributor Author

@aditya172926 aditya172926 commented Feb 21, 2022

@glemaitre can you have a look and let me know an update on this PR.

@aditya172926 aditya172926 requested a review from glemaitre Feb 24, 2022
@lesteve
Copy link
Member

@lesteve lesteve commented Feb 25, 2022

I pushed a commit into your branch tackling #22461 (comment). I think this should get rid of the warning.

About your error, if you want to build from source you should look at: https://scikit-learn.org/stable/developers/advanced_installation.html#building-from-source

@lesteve
Copy link
Member

@lesteve lesteve commented Feb 25, 2022

Looks good, merging, thanks!

@lesteve lesteve changed the title DOC FIX notebook-style for plot_stock_market DOC: use notebook-style for plot_stock_market.py Feb 25, 2022
@lesteve lesteve merged commit bd10a1b into scikit-learn:main Feb 25, 2022
29 checks passed
thomasjpfan added a commit to thomasjpfan/scikit-learn that referenced this issue Feb 26, 2022
@aditya172926 @lesteve @thomasjpfan
DOC: use notebook-style for plot_stock_market.py (scikit-learn#22461)
0336d82 
Co-authored-by: Loïc Estève <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre

Assignees
No one assigned
Labels
Documentation Quick Review
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
3 participants
@aditya172926 @glemaitre @lesteve