Skip to content
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

Finding a place for rendered notebooks #322

Closed
kolibril13 opened this issue Oct 27, 2021 · 5 comments
Closed

Finding a place for rendered notebooks #322

kolibril13 opened this issue Oct 27, 2021 · 5 comments

Comments

@kolibril13
Copy link
Contributor

As discussed in #88 (@zivy , @blowekamp ), it would be nice to update the notebook infrastructure.

These could either live on a new readthedocs page which replaces http://insightsoftwareconsortium.github.io/SimpleITK-Notebooks/ or on the already existing page: https://simpleitk.readthedocs.io/en/master/index.html
Some thoughts.

Advantages of both approaches:

  • No security warning on the website anymore
  • search bar
  • easy to copy cells

Advantages of separate readthedocs page

  • "furo" theme can be used, which goes really nice with the notebooks (see here) A neat subchapter navigation bar will be visible on the right ride.
  • When using the search function, one will find only pure python functions that are called in the notebooks, and not other references that one might not want to see (e.g. C++, Java references that are also there).
  • Rendering the notebooks and pushing them to readthedocs is independent of sitk releases.

Advantages of combining to simpleitk.readthedocs.io

  • everything in one place, easier to find for users.

Disadvantages of combining to simpleitk.readthedocs.io

  • Rendered notebooks won't look very nice with the "sphinx_rtd_theme".
  • Rendered notebook versions might not match the version of sitk, in case that they are not rendered every time there is a new release.
  • As the rendered notebooks contain images, these will be also added to the github history, which will increase the size of the repo. There it might be better to have one separate example notebook repo of big size and one small repo with core functionality.
  • Not rendered notebooks have to be manually copied from this repo (SimpleITK-Notebooks) to the https://github.com/SimpleITK/SimpleITK repo .

With these arguments, I'd prefer the separate solution. But I might have missed important arguments, so feel free to add your thoughts!

Additional thought:

I think
http://insightsoftwareconsortium.github.io/SimpleITK-Notebooks
should get updated after the transition. All notebooks should get removed, and only a link to the new rendered notebooks should remain.

@zivy
Copy link
Member

zivy commented Oct 27, 2021

My 2cents:

  • I like the proposed theme's look and feel. Looks much better than the current notebook renderings.
  • The notebooks do not follow the SimpleITK release schedule and are valid across multiple minor releases, so likely should not be integrated into the core SimpleITK RTD.
  • Addressing the "all documentation in one place" concern, just point from the core RTD to the new notebooks location. In the "Getting Started" section we already point to the current notebooks site. Maybe move this to a more prominent location, last entry in the table of contents after Examples?

Before proceeding, we should iron out kinks in the proposed rendering:

  1. Ensure that the executed notebooks look as expected (not crucial, but I noticed the "launch binder" badge lost the hyperlink so clicking it does nothing).
  2. Replace the github pages with a redirect to the new notebook RTD site.
  3. Update the main SimpleITK site to point to the new RTD site.

@kolibril13, can you update the site you created with the rendering of the executed notebooks so that we can see how those look? This will be our third attempt at automating the rendering of executed notebooks, hopefully this time it works.

@kolibril13
Copy link
Contributor Author

I like the proposed theme's look and feel. Looks much better than the current notebook renderings.

I am glad about that, let's stick with this solution then.

just point from the core RTD to the new notebooks location

Good idea!

not crucial, but I noticed the "launch binder" badge lost the hyperlink

True, thanks for noting.
As some notebooks don't have a binder link at all (e.g. https://okapi3--3.org.readthedocs.build/en/3/36_Microscopy_Colocalization_Distance_Analysis.html)
What about first removing all binder links and then re-creating all binder links and making them clickable and some point in future?

can you update the site you created with the rendering of the executed notebooks so that we can see how those look?

Yes, I can do that after #323 is merged, so that the notebooks are reliable.

Also, I've noted that here https://okapi3--3.org.readthedocs.build/en/3/02_Pythonic_Image.html#Mathematical-Operators are two cells that should be rewritten.
As I prefer working with small prs, I think this is a task I would take after #323 is merged.

@zivy
Copy link
Member

zivy commented Oct 28, 2021

#323 will be merged later today, once it passes the testing.

With respect to the binder badge links, let's leave it as is for now (removing and then restoring just creates noise).

@kolibril13
Copy link
Contributor Author

kolibril13 commented Oct 28, 2021

The binder links are working there at nbsphinx docs, I think we can later just copy their approach:
https://nbsphinx.readthedocs.io/en/0.8.6/executing-notebooks.html

@kolibril13
Copy link
Contributor Author

As https://simpleitk-notebooks.readthedocs.io/ is now online, I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants