-
Notifications
You must be signed in to change notification settings - Fork 8
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
Improved docs for NDSL examples #63
Improved docs for NDSL examples #63
Conversation
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 am for removing all results here and encoding them in the markdown and/or rendering the plots.
bb539e5
to
97ee945
Compare
- Use boilerplate from ndsl.boilerplate - Fix typos & broken links in notebooks - Added README with quickstart guide
97ee945
to
8c6b4e0
Compare
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.
Some notes/questions inline
For some reason, this file was had the executable flag, marking it an executable under linux-based filesystem. Since notebooks aren't directly executed and rather read by Jupyter (or whatever platform), this is unnecessary. Other notebooks didn't have the executable flag (which is what I would expect).
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.
Looking good, merging this
Description
Tutorials are in important step in on-boarding new library users. While working through the examples, I noticed a couple of typos and broken links. In the bigger picture, we figured that examples should use the boilerplate code that the middleware already exposes in
ndsl.boilerplate
. I've also added a README file with a little bit of context and a quickstart guide to get people up and running with minimal overhead. And we agreed to remove the output currently stored in the Jupyter notebooks. This was one in the first commit such that we can see the actual changes when looking at the other commits.In summary
plot_filed_at_kN
fromexamples/NDSL/basic_boilerplate.py
tondsl/boilerplate.py
. This addsmatplotlib
to the list of dependnecies.ndsl.boilerplate
.examples/NDSL/*_boilerplate.py
examples/NDSL/README.md
with a bit of context.It is recommended to exclude the first commit when looking at the diff, e.g. this view is much nicer to look at.
Regarding the previous addition of a
.tool-versions
file a python version manager: With the install instructions as simple aspip install ndsl[demos]
, I don't think we need this complexity anymore. The middleware's README already specifies the need for python 3.11. How developers ensure this should be up to them.Fixes: N/A
How Has This Been Tested?
I ran all the notebooks locally against a local version of NDSL.
Checklist: