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

ENH Add python code for notebook used in the Pipeline video #494

Merged
merged 11 commits into from
Dec 10, 2021

Conversation

ArturoAmorQ
Copy link
Collaborator

Fixes #465.

I think the best place for the video would be somewhere between Exercise M1.05 and the first wrap-up quiz.

The idea is that the video and notebook will be displayed simultaneously within a split webpage. This may require FUN changes.

Copy link
Collaborator

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM. I assume that the goal of keeping this notebook is to:

  • make it possible to point the users who ask for just in case they want to reproduce it locally,
  • archive the script in the event we want to rerecord this video with an update in the UI of of scikit-learn in a future release.

But it should not be included in the Table of Content of the jupyter book or the navigation menus of FUN.

python_scripts/video_pipeline.py Outdated Show resolved Hide resolved
python_scripts/video_pipeline.py Outdated Show resolved Hide resolved
python_scripts/video_pipeline.py Outdated Show resolved Hide resolved
ArturoAmorQ and others added 3 commits December 8, 2021 16:13
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
@ArturoAmorQ
Copy link
Collaborator Author

Done, thanks!

Copy link
Collaborator

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I am fine with merging this PR and iterating with other PRs focused on fixing the CSS if you prefer.

@ArturoAmorQ
Copy link
Collaborator Author

I am fine with merging this PR and iterating with other PRs focused on fixing the CSS if you prefer.

I can start embedding the videos on the rest of the slides but I may need help

@ArturoAmorQ
Copy link
Collaborator Author

The progress bar is not showing, do you think it is just hidden because of the size of the video, @ogrisel ?

@ogrisel
Copy link
Collaborator

ogrisel commented Dec 10, 2021

The progress bar is not showing, do you think it is just hidden because of the size of the video, @ogrisel ?

Yes, because if the iframe css in sklearn_mooc.css.

@ogrisel
Copy link
Collaborator

ogrisel commented Dec 10, 2021

I can start embedding the videos on the rest of the slides but I may need help

Please do so and I will open a PR to fix the CSS next week.

@ogrisel ogrisel merged commit 5eafcd3 into INRIA:master Dec 10, 2021
@ogrisel
Copy link
Collaborator

ogrisel commented Dec 10, 2021

Let's merge and iterate in other PRs.

github-actions bot pushed a commit that referenced this pull request Dec 10, 2021
@ArturoAmorQ ArturoAmorQ deleted the video_pipeline branch March 11, 2022 13:24
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.

Add video of HTML representation of Pipeline
4 participants