Skip to content

Conversation

@paskino
Copy link
Contributor

@paskino paskino commented Jul 8, 2025

Description

After discussion with @hrobarts we decided to not remove the original conda install commands as the new proposed one didn't seem simple.

Also the old text made clear what the role of the additional dependencies is and even how to get ASTRA for CPU or GPU.

We added the new command pointing to the environment file with the instruction that it was installing the software necessary for CIL-Demos with tested versions.

Checklist

  • I have performed a self-review of my code
  • I have added docstrings in line with the guidance in the developer guide
  • I have updated the relevant documentation
  • I have implemented unit tests that cover any new or modified functionality
  • CHANGELOG.md has been updated with any functionality change
  • Request review from all relevant developers
  • Change pull request label to 'Waiting for review'

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 (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

@paskino paskino changed the title added Table of dependencies versions Added table of dependencies versions to README Jul 8, 2025
@gfardell
Copy link
Member

Do we want to also update the conda instructions to use the env files here:
TomographicImaging/scripts#3

Hannah has drafted something here:
https://github.com/TomographicImaging/Build-scripts/blob/2709ecd3ba2c3b30418ad71d38094c3e16450d28/README.md#installation-with-conda

@paskino paskino requested review from casperdcl and hrobarts July 30, 2025 13:21
@paskino paskino marked this pull request as ready for review July 30, 2025 13:21
hrobarts and others added 2 commits August 5, 2025 11:00
Co-authored-by: Casper da Costa-Luis <[email protected]>
Signed-off-by: Hannah Robarts <[email protected]>
Signed-off-by: Casper da Costa-Luis <[email protected]>
Signed-off-by: Casper da Costa-Luis <[email protected]>
Co-authored-by: Casper da Costa-Luis <[email protected]>
Signed-off-by: Hannah Robarts <[email protected]>
Copy link
Member

@gfardell gfardell left a comment

Choose a reason for hiding this comment

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

It seems very clear to me! I think it's a good change. The tested versions we can update as we iterate.

@hrobarts hrobarts requested a review from casperdcl August 8, 2025 15:18
Copy link
Member

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

well that was easy

Signed-off-by: Edoardo Pasca <[email protected]>
Signed-off-by: Edoardo Pasca <[email protected]>
@paskino paskino merged commit e099bd3 into master Aug 12, 2025
1 check passed
@paskino paskino deleted the readme_dependencies_table branch August 12, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tested matrix to readme

5 participants