Use the last image in a notebook as the default thumbnail

[angus-g]

The nbsphinx_thumbnail_default configuration key controls whether the default thumbnail for a notebook is the default icon, if it is 'none' (the default). If it is 'last' and a notebook has any valid image outputs, the last of these will be used for the thumbnail.

This is to get back to the default behaviour offered by sphinx_nbexamples. I'm not sure if it's possible to showcase this in the documentation, since it requires changing a config switch, rather than being per-notebook.

There is still a little snag here: because this setting provides a value for nbsphinx_thumbnail for all notebooks with an image, the custom thumbnails in the nbsphinx_thumbnails config mapping are overwritten. I could also add another configuration key to determine whether the notebook or the config file is the authoritative source for the thumbnail, e.g.

Example diff

diff --git a/src/nbsphinx.py b/src/nbsphinx.py
index 7cd32a3..62e5b02 100644
--- a/src/nbsphinx.py
+++ b/src/nbsphinx.py
@@ -1182,8 +1182,9 @@ class NotebookParser(rst.Parser):
         if resources.get('nbsphinx_widgets', False):
             env.nbsphinx_widgets.add(env.docname)

-        env.nbsphinx_thumbnails[env.docname] = resources.get(
-            'nbsphinx_thumbnail', {})
+        if env.config.nbsphinx_thumbnail_overwrite or env.docname not in env.config.nbsphinx_thumbnails:
+            env.nbsphinx_thumbnails[env.docname] = resources.get(
+                'nbsphinx_thumbnail', {})


 class NotebookError(sphinx.errors.SphinxError):
@@ -2480,6 +2481,7 @@ def setup(app):
     app.add_config_value('nbsphinx_widgets_options', {}, rebuild='html')
     app.add_config_value('nbsphinx_thumbnails', {}, rebuild='html')
     app.add_config_value('nbsphinx_thumbnail_default', 'none', rebuild='html')
+    app.add_config_value('nbsphinx_thumbnail_overwrite', True, rebuild='html')
     app.add_config_value('nbsphinx_assume_equations', True, rebuild='env')

     app.add_directive('nbinput', NbInput)

Although this feels a little gross, I don't quite understand the ordering of env vs. config. I guess otherwise this would go in at the env_merge_info stage...?

[mgeier]

Thanks for this PR!

This fits very well to the other gallery change: #706

I think if we do this we can drop the backwards-compatibility option.
It makes sense to use the last image, and if somebody really wants the generic default image, they can still select it with nbsphinx_thumbnails. Which brings me to ...

There is still a little snag here: because this setting provides a value for nbsphinx_thumbnail for all notebooks with an image, the custom thumbnails in the nbsphinx_thumbnails config mapping are overwritten.

Yeah, that would be unexpected.

Which got me thinking: what happens when a thumbnail is both specified in a notebook and in nbsphinx_thumbnails?
Currently, the notebook metadata seems to win, but I don't think that's obvious.

I could also add another configuration key to determine whether the notebook or the config file is the authoritative source for the thumbnail

I don't think an additional option is necessary for this.

I think if the priority is obvious (i.e. specified vs. implied), the obvious thing should happen, i.e. the user-specified image should be shown.
However, if it's not obvious (i.e. specified in notebook vs. conf.py), a warning should be generated.

Does that make sense?

[angus-g]

I think if the priority is obvious (i.e. specified vs. implied), the obvious thing should happen, i.e. the user-specified image should be shown.
However, if it's not obvious (i.e. specified in notebook vs. conf.py), a warning should be generated.

Agreed, that's a good way to handle it.

[angus-g]

Okay, I finally found where in the code the priority is actually handled! The previous behaviour was to prefer thumbnails specified in the notebook to those in the config (and this was denoted with a comment). This is probably still reasonable (e.g. if there was a fairly wide pattern). I've slotted the implicit thumbnail in below these options, as a fallback before resorting to the default.

[mgeier]

Thanks for the update!

Do you think the nbsphinx_thumbnail_default option is really necessary?

I think it would be better to always take the last image as a fallback (unless there is no image at all, in which case the "no thumbnail" image would be used).

It would be great to have a new example notebook that shows the new behavior.
And the "cell tag" example (https://nbsphinx.readthedocs.io/en/0.8.12/gallery/cell-tag.html) should be changed to not use the last image.
If you don't have time to create examples, let me know and I can do it after merging this PR.

[angus-g]

Do you think the nbsphinx_thumbnail_default option is really necessary?

I think it would be better to always take the last image as a fallback (unless there is no image at all, in which case the "no thumbnail" image would be used).

No problem, I can change it to work this way. I guess I didn't want to break anything existing by changing the default thumbnail to an image out of the notebook, but since the default thumbnail has changed to be pretty glaring it's fine now! It's also possible to just wildcard back to a default anyway.

It would be great to have a new example notebook that shows the new behavior. And the "cell tag" example (https://nbsphinx.readthedocs.io/en/0.8.12/gallery/cell-tag.html) should be changed to not use the last image.

Given that there won't be a specific configuration knob needed, I'm happy to make these changes now.

[angus-g]

Whoops, my example didn't quite work as intended! The image shown by display doesn't seem to get picked up as a candidate to be a thumbnail, so it's still defaulting to the plot... Maybe because I used an SVG which doesn't embed? Anyway, switched to a regular plot and it works fine now.

[mgeier]

Thanks for the update!

However, there seems to be a bug somewhere, because the cell-tag thumbnail in https://nbsphinx--707.org.readthedocs.build/en/707/gallery/gallery-with-nested-documents.html shows the last image, not the one that claims to be tagged.

Also, the image in the code-cells thumbnail in https://nbsphinx--707.org.readthedocs.build/en/707/gallery/gallery-with-links.html#Gallery-With-Links-(HTML-only) doesn't seem to exist.

Maybe because I used an SVG which doesn't embed?

That's an interesting situation ... would you expect this to work?
Anyway, if we want to, we can tackle this in a separate PR after this one.

[mgeier]

Shouldn't be the default behavior the first in this list?

[mgeier]

@angus-g Did you see my previous comment? Do you know what's going wrong in the cell-tag example?

It would be great if we could fix this and then merge this PR.

If you don't have time, no problem, but please let me know so I can have a stab at fixing this.

[mgeier]

I don't want to be too impatient, but I took the liberty of using your commits and appending my own changes here: #717.

I think this should fix most of my comments above.

However, the image from code-cells.ipynb still doesn't work. It should be the image from the YouTube video. I'll keep that as it is for now, maybe at a later point we can solve that problem.

[mgeier]

I have merged #717, thanks for initiating this!

I will make a new release soon. This way, the breaking changes are published and people can start using it.
If you find that something doesn't work or if you have suggestions for improving the docs, please make further PRs!