-
Notifications
You must be signed in to change notification settings - Fork 51
docs: add option to open notebook directly in Colab #126
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
Conversation
| } | ||
|
|
||
| COLAB_INSTALL_CELL = """\ | ||
| # Install data-designer and dependencies |
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 one of those "parrot comments", I think. Maybe add this to the markdown note that appears above this cell?
Something like:
"Run the cells below install Data Designer and set up the environment for Google Colab."
Also, a couple questions:
- Do we want to use
-qor put%%captureat the top of the cell? We used the latter at Gretel, but if-qdoes the same, that's cool. - Should we pin the version on the install?
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.
Changed!
Re questions
- I like
-qbecause if there are issues they still show up, but we could use%%capture, don't have a strong opinion. - I've added a snippet that uses
>=the current version, see what you think. I'm using whateverdata_designer.__version__outputs but doing -1 on the minor, since I think this is always a dev version? I.e. we won't do a PR on a tagged commit, we tag for releasing after the PR 🤔
| COLAB_API_KEY_CELL = """\ | ||
| # Set up NVIDIA API key from Colab secrets | ||
| import os | ||
|
|
||
| from google.colab import userdata | ||
|
|
||
| os.environ["NVIDIA_API_KEY"] = userdata.get("NVIDIA_API_KEY")""" |
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.
Should we do this or use getpass and ask for their API key? @kirit93, thoughts on what's the preferred flow?
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 did a try/except, which looks for the envvar inside userdata, but if it isn't there it asks for it
|
|
||
|
|
||
| def find_import_section_index(cells: list[NotebookNode]) -> int: | ||
| """Find the index of the 'Import the essentials' markdown cell.""" |
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.
Does this mean we always have to have this markdown to detect the section? Wondering if from data_designer.essentials import is a more future-proof check?
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.
We can do that, but then you will likely have at least 1 or more markdown cells before it, hard to know. We can assume it's 1 but I don't know if that will always be the case.
Maybe right after the 1st markdown cell? And then we make sure that the 1st one is always title plus some description.
johnnygreco
left a comment
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 neat @andreatgretel – thank you!
Just had a few questions to think through.
johnnygreco
left a comment
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.
🛸
701f304 to
e94a5a8
Compare
docs/scripts/generate_colab_notebooks.pythat injects new cells and converts .py notebooks (without executing)generate-colab-notebooks