-
Notifications
You must be signed in to change notification settings - Fork 910
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
[READY] Add new notebook example to docs about adding Kedro to a notebook project #3128
Conversation
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like how this is shaping up, great job @stichbury !
Main comments:
- I really like the flow. One outstanding point is how do we make the user download the code. I propose we tell them to clone a repository, or even open it on Gitpod. Could be
kedro-academy
, could be this one. I'm -0.5 on adding thedocs/examples
, but not completely opposed to it if we think it's the right thing to do. - I opened a PR with some proposed code changes Add new notebook tweaks #3133 including using the config loader with the current directory, so the user doesn't have to create
conf/{base,local}
. We can end there, or we can actually create the dirs, but in a second step IMHO. - Some parts at the end of the tutorial repeat a lot of code. I'm thinking whether we could assume that the user has the context and we can make the final code blocks more terse (rather than fully self-contained). There's potential of creating some confusion, but at the moment this all feels like too much text.
* Minor code changes to notebook Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Partially apply black to code samples Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> --------- Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really amazing work @stichbury 🤩 I left two minor comments, but other than that all good from my side.
docs/source/notebooks_and_ipython/notebook-example/add_kedro_to_a_notebook.md
Show resolved
Hide resolved
docs/source/notebooks_and_ipython/notebook-example/add_kedro_to_a_notebook.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit long, but absolutely wonderful nonetheless 💯 Well done @stichbury !
Two final questions:
- do we need both the
.ipynb
and the.md
? I reckon having these makes it easier for distracted users that don't install jupytext to just open the.ipynb
and go on with their lives. The cost is having the content twice and having to sync it ourselves. Not a big deal, just wanted to make it explicit. - do you plan to keep the example under
docs/source/notebook-example
, or is this an intermediate step towards having it on https://github.com/kedro-org/kedro-academy/ ? I think this is excellent tutorial material (welp, I've been delivering similar tutorials all year!) and would be a shame to not have it there. but if there's a rationale to keep it here, then fine.
Apart from these two, LGTM 👍🏽
Yes, I wrestled over this but I don't want to force people to use jupytext since this is onboarding type content. The goal is to have a notebook ready to go, but it does come at the expense of us having to understand how to work with the two files. I did write a README for this but removed it. Maybe I'll write a comment in the markdown file to explain how to make updates before I merge this.
I want to keep this in the docs repo. Reason being that otherwise part 1. becomes difficult. We have to copy the markdown across repos, or the notebook, and that just gets annoying. At least with both in the same directory the code and example are side by side, and to make that happen and be able to build the docs, it has to be here. Also, I looked at the Thanks for asking: making this explicit is a good decision record. |
Description
Answers #2855
Development notes
Introduces a new example with a markdown that mirrors a full runnable notebook in a downloadable directory.
Incrementally adds Kedro features to a notebook project for spaceflights.
Take a look at the built docs or review the markdown. PLEASE DO NOT EDIT THE NOTEBOOK!
Checklist
RELEASE.md
file