-
Notifications
You must be signed in to change notification settings - Fork 3
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
Improve and expand the tutorials #3
Conversation
src/pipeline.ipynb
Outdated
"try:\n", | ||
" import fondant\n", | ||
"except ImportError:\n", | ||
" logging.warning(\"Please install Fondant from the `requirements.txt` at the root of this repository\")" |
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 would add a cell to install the requirements.txt from the notebook. To make it really bulletproofed.
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.
Nice improvements! Left a few suggestions
Co-authored-by: Philippe Moussalli <[email protected]>
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.
Thanks Robbe! I will update the datacomp repo based on this
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.
Thanks @RobbeSneyders. Last small nitpick for M1 users.
"\n", | ||
"from pathlib import Path\n", | ||
"\n", | ||
"DockerCompiler().compile(pipeline=pipeline, output_path=\"docker-compose.yml\")\n", |
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.
Could we still add this to make it work on a M1 machine:
"DockerCompiler().compile(pipeline=pipeline, output_path=\"docker-compose.yml\")\n", | |
"import os\n", | |
"os.environ["DOCKER_DEFAULT_PLATFORM"]="linux/amd64"\n", | |
"DockerCompiler().compile(pipeline=pipeline, output_path=\"docker-compose.yml\")\n", |
I made some further improvements to both the notebook and the README. I would like us to update all examples to match this.
Some changes: