-
Notifications
You must be signed in to change notification settings - Fork 2
chore: Updated demos and readmes for wheel release #129
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
|
Key consideration @Tom-Owen-ONS for your review: currently there's a lot of duplication in that roughly the same install instructions section is in every notebook. Whereas we could put one set of install instructions in the /DEMO/readme.md so that the notebooks have less content. However, if we keep it the way it is currently proposed then the notebooks are much more self-contained, i.e. the user doesn't need to download the whole DEMO folder, and each notebook has complete instructions on how it should be run. |
Tom-Owen-ONS
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.
Key consideration @Tom-Owen-ONS for your review:
currently there's a lot of duplication in that roughly the same install instructions section is in every notebook. Whereas we could put one set of install instructions in the /DEMO/readme.md so that the notebooks have less content.
However, if we keep it the way it is currently proposed then the notebooks are much more self-contained, i.e. the user doesn't need to download the whole DEMO folder, and each notebook has complete instructions on how it should be run.
I think for notebooks I prefer having the installation instructions inside as they are: I kind of see them as self-contained and data scientists/testers who use them might treat them more like that as well.
Tom-Owen-ONS
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.
Notebooks all run on a cloud workstation for me and the updated docs make sense (just the little note on the README that is up to you to be honest). Haven't tried on an on-net due to issues we discussed.
Thanks for this!
Tom-Owen-ONS
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.
looks great!
resolves #128
✨ Summary
These changes add more detailed instructions for each DEMO notebook, on how to set up the virtual environment with UV or PIP and install from wheel reversion release with the appropriate optional dependency installs for each notebook. It also adds some additional setup requirements around DEMO datasets, ipykernel and how to access those
📜 Changes Introduced
✅ Checklist
terraform fmt&terraform validate)🔍 How to Test
Trying out the demo notebooks and following the corresponding instructions, possibly in a fresh folder environment with just the single ipynb file downloaded would be good a good way to determine if the instructions are sufficient for user to follow through and acquire any additional data that is needed