Closed GenevieveBuckley closed 3 years ago
Love this idea @GenevieveBuckley! I think your PR https://github.com/napari/napari/pull/1269 makes all the world of difference :-) will be so nice to have all this be autogenerated etc.
@tlambert03 @jni @kne42 any thoughts on this/ advice before getting started? Is something like https://jupyterbook.org/intro.html overkill or could it easy to integrate with the notebooks too? @GenevieveBuckley do you have any experience with jupyterbook? I've only heard good things about it, never used it myself.
@GenevieveBuckley if you were interested in taking the lead on this conversion I'd love that too!!
I hate notebooks for authoring, because they are terrible to review on GitHub and terrible to do diffs on to make improvements. JupyterBook + Markdown/MyST authoring is one option, and I'd be totally fine with it, but I think easier is sphinx-gallery, which just added support for animations, too. It's what we use for skimage, see e.g. this example:
Here's what the source looks like — it's a runnable Python script:
Finally, we have to figure out the sample data issue. Talley's tutorial needs a pretty big dataset to start with!
Sorry, I forgot to say about sphinx gallery: it generates both blank and completed Jupyter notebooks that you can download. You can find those links at the bottom of the page I linked to.
To clarify because I think that might have come off a bit negative. I absolutely think that we should have executable tutorials and that they should be executed with each napari release and that they should be downloadable as notebooks! So thanks for opening the issue @GenevieveBuckley! But I would like the source format to not be notebooks.
@sofroniewn, no I don't have any experience with JupyterBook. I'll have to look more at that, thanks
@jni, I don't think you're being negative at all!
To clarify because I think that might have come off a bit negative. I absolutely think that we should have executable tutorials and that they should be executed with each napari release and that they should be downloadable as notebooks!
What you describe here is right. What I care about is picking the best way to do this, not necessarily the first thing that popped into my head.
I agree that notebooks are a pain to review on GitHub and do diffs on.
My concern with sphinx gallery scripts is that napari python scripts and napari notebooks seem incompatible.
napari.gui_qt()
context manager, or else hit a RuntimeError.with napari.gui_qt():
viewer = napari.Viewer()
# ... do stuff
Am I wrong about this?
This is also the reason I don't really like the format of the napari examples. It's an extra step away from what you want the user to be doing, which imo is running napari interactively from ipython/jupyter. I'd like to see that style of use be prioritized and encouraged by all the examples & docs here.
Another tool we might want to use is jupytext which would let us store the notebooks at markdown and then convert them to jupyter notebooks. I've heard of others using it for the functionality we need. Maybe if we poke around we can find some examples, I can't remember any off the top of my head.
I'm also excited about sphinx-gallery
too, I love the scikit-image tutorials and often copy and paste code out of them, though @GenevieveBuckley is right about the napari.gui_qt()
potentially being confusing.
I will add that for my personal use, I like scripts. I find myself making little napari launch scripts and the using the built-in napari console much more frequently than launching from notebooks, so I wouldn't necessarily assert that notebook usage is what we want a user to be doing, they are just different and it is great that we support both.
We do want to be conscious though of what a beginner finds least confusing and easiest to use when presenting our tutorials, and I'm absolutely fine if the majority feels that is notebooks. I teach from notebooks for example, so definitely see the rationale there.
Would nbdime make you feel better about notebook diffs, @jni?
Would nbdime make you feel better about notebook diffs, @jni?
No, it's kind of a pain to work with and super non-standard. Plus it doesn't solve the GitHub problem.
From the meeting today:
.py
script (so we don't need the with napari.gui_qt()
context blocks everywhere).I'll look into both these suggestions and check back in next week.
one idea for the first point, is to add this to napari.__main__
(from zulip)
if len(args.images) == 1 and args.images[0].endswith('.py'):
with gui_qt(startup_logo=False):
exec(open(args.images[0]).read())
else:
with gui_qt(startup_logo=True):
view_path(
args.images,
stack=args.stack,
plugin=args.plugin,
layer_type=args.layer_type,
**kwargs,
)
One thing to note: if you use the suggestion by @tlambert03 above and call a napari script that also includes the with napari.gui_qt()
context block, then the terminal will hang after the napari window is closed. I think the close signal is visible only to the innermost qt event loop, and the outer one never receives a stop signal.
How much of a problem is this for us? Obviously what I've just described is not intended use, but it'll definitely pop up when users start to play with these tools.
@GenevieveBuckley I think that's a pretty significant problem! I hope that we can engineer around it though...
@tlambert03 I want to repeat @0x00b1's advice from Zulip here to use runpy(filename)
rather than exec(open(filename).read())
.
@GenevieveBuckley I think that's a pretty significant problem! I hope that we can engineer around it though...
I agree, I don't like it.
@tlambert03 I want to repeat @0x00b1's advice from Zulip here to use
runpy(filename)
rather thanexec(open(filename).read())
.
That should be runpy.run_path(filename)
Definitely solvable... I feel like it was working when I tried it at one point. Let me know if you run out of leads
Definitely solvable... I feel like it was working when I tried it at one point. Let me know if you run out of leads
You were able to nest gut_qt
context managers & have them all quit at the appropriate time?
here's a solution:
# event_loop.py
def gui_qt(*, startup_logo=False):
if not app:
...
else:
app.nested = True # add this
# __main__.py
...
# can probably do better than this line
if len(args.images) == 1 and args.images[0].endswith('.py'):
import runpy
with gui_qt(startup_logo=False) as app:
runpy.run_path(args.images[0])
if getattr(app, 'nested', False):
sys.exit()
else:
...
That works really nicely @tlambert03, do you want to open a PR with that on the main napari repo?
I had been mucking around with the contextlib ExitStack but couldn't integrate it with runpy, so this feels much cleaner.
Would nbdime make you feel better about notebook diffs, @jni?
No, it's kind of a pain to work with and super non-standard. Plus it doesn't solve the GitHub problem.
Is ReviewNB a solution for this then? https://www.reviewnb.com/
It's free for all open source projects, and more recently free for education use too (includes private repos).
I also want to link to nbQA (https://github.com/nbQA-dev/nbQA) which is a suite of tools you can use to add pre-commit hooks to jupyter notebooks (it includes tools like black, isort, mypy).
I'd like to propose that we change the napari tutorials repository to hold example notebooks, instead of example markdown files, which get rendered in html on the tutorial website.
Since we're adding functionality for displaying screenshots in-line in notebooks, this seems like it could be the perfect time (especially since there's relatively few examples in here now).
This would have several advantages:
Some other projects using notebooks to generate docs: