-
Notifications
You must be signed in to change notification settings - Fork 4
Revised before you begin page #108
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
revised my first draft of the page, moving onto embedding cells from the before you begin page to other pages regarding jupyter notebooks |
Great timing! I'd started to review your initial commit but then got sidetracked. I'll take a look at this updated version now, @p-m-s-f. |
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.
Thank you for pushing an updated PR, @p-m-s-f! This looks much cleaner than the first version, but it now actually looks too sparse to actually work. I don't think this notebook actually could run, can it? I think we will need at least the code cells you removed back in the notebook, as they include important code for the notebooks to be executable. After that, we can figure out how to simplify the English instructions. I do laud your effort to clean things up, though!
Here's a screenshot in this comment to show what we might need back in. At a minimum, the code cells need to be included.

These instructions are from https://github.com/validmind/demo-environment/blob/main/demo_files/Quickstart_Customer%20Churn_full_suite.ipynb.
As for the English instructions, I don't think we want the current ones back in verbatim, as that is quite a bit of text. So the magic trick will be to figure out how little instructions we can get away with (and link to the docs for the rest, as you have done).
As an aside, you might need to merge main
into your PR branch or rebase your commits on top of main
(with some assistance from me), as you appear to be missing some stuff. E.g. your PR looks to remove the PR template Sydney added last week and the CI checks — we should not be deleting these. I'm not 100% sure what happened, but happy to take a look at it with you tomorrow.
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.
Getting closer! I left a comment about adding a tip to create a documentation project (you need one to initialize the framework). I think that is the only bit that we definitely still need.
As part of your PR, can you add and test out your steps to make sure they work? E.g. you could update site/notebooks/nlp/nlp_sentiment_analysis_demo.ipynb
with your before-you-begin snippet and then run all the cells.
Co-authored-by: Nik Richers <nik@validmind.ai>
i created a documentation project but i wasn't able to get the code working-- perhaps this is something to return to on tuesday? i'm not the most familiar with the developer framework. i've included a copy of the customer churn notebook with my before you begin section, it should have the error messages intact. |
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 updates for this pull request, @p-m-s-f!
Just parking some notes here for discussion with you:
-
Minor text edits (product names are proper nouns, add some info about why creating a documentation project matters, don't send people to Support; have just one tip)
-
Figure out why this PR seems to delete some stuff we definitely need (e.g. the pull request template)
We can chat on Zoom a bit today, if you like, but generally this PR is very close to where it needs to be before we roll out the changes.
Replaced by #111. |
Responding to this story.
I've moved onto the second draft. At Nik's request, I've greatly shortened the length of the page, and removed some references to Google Colab. I've also listened to Anil's suggestion to link to the page describing how to create documentation projects.
Also, I spotted (and fixed) a typo on our customer churn quickstart page.