Skip to content

Update all notebooks to be rendered - #34

Open
lauramurgatroyd wants to merge 6 commits into
mainfrom
cil_v26
Open

Update all notebooks to be rendered#34
lauramurgatroyd wants to merge 6 commits into
mainfrom
cil_v26

Conversation

@lauramurgatroyd

@lauramurgatroyd lauramurgatroyd commented Jul 28, 2026

Copy link
Copy Markdown
Member

Contribution Overview

  • Tested all notebooks with CIL v26.0.0 and documented this
  • Added links to data in Diondo notebook and listed joblib as a requirement, and where to download it
  • Rendered all notebooks
  • Removed some duplicate imports

Contribution Notes

Please read and adhere to the developer guide and local patterns and conventions.

  • The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in CIL and the CIL Reader Showcase (the Work) under the terms and conditions of the Apache-2.0 License
  • I confirm that the contribution does not violate any intellectual property rights of third parties

@lauramurgatroyd
lauramurgatroyd marked this pull request as ready for review July 28, 2026 08:30
@lauramurgatroyd
lauramurgatroyd requested a review from gfardell July 28, 2026 08:38
Comment thread 000_NXTomoReader_Steelwire.ipynb

@Neonbluestoplight Neonbluestoplight left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are mostly nitpicks and tiny enhancements rather than big fixes. I have checked for typos, whether the dataset links work or not and general rendering.

All looks good to me but would like to hear what you say and if anything would require changing

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Minor nitpick not really an issue. Maybe could add titles to differentiate plots (This is just shows as an example but is a case for all the plots shown).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Missing attribution on datasets when compared to other notebooks

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall suggestions

These are more or less nitpicks rather than things that need to be done:

  1. Image

No data attribution when compared to other notebooks (as in crediting the providers). May be unnecessary since link to Zenodo covers it.


  1. Image

Not sure what this means:

Here demo how to read in Nikon Data, perform FDK reconstruction, and perform model based image reconstruction utilizing a primal dual hybrid gradient.

Maybe "Demo on how to...."


  1. Image

Maybe add code blocks to show2D since it has been done with all the other methods.


Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Image

flat instead of fla


  1. Image

The [Data set] part seems to be for a link which is not present (maybe it is just the Zenodo link?)


  1. Image

Should be a code block (Text block after cell 13)


Comment thread 004_DiondoReader.ipynb

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Maybe unnecessary but CIL version used for development of notebook not mentioned:
Image
  1. Image

2 identical plots with no indication about a need?

Comment thread README.md
conda env create -f https://tomographicimaging.github.io/scripts/env/cil_demos.yml
```

Please note, although many were developed with earlier versions of CIL, all notebooks have been tested with CIL v26.0.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Please note, although many were developed with earlier versions of CIL, all notebooks have been tested with CIL v26.0.0
> [!NOTE]
> Although many were developed with earlier versions of CIL, all notebooks have been tested with CIL v26.0.0

Will render as below

Note

Although many were developed with earlier versions of CIL, all notebooks have been tested with CIL v26.0.0

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants