Skip to content

Make dxchange and olefile optional requirements#2209

Merged
hrobarts merged 10 commits into
masterfrom
io_run_reqs
Nov 20, 2025
Merged

Make dxchange and olefile optional requirements#2209
hrobarts merged 10 commits into
masterfrom
io_run_reqs

Conversation

@hrobarts
Copy link
Copy Markdown
Contributor

@hrobarts hrobarts commented Aug 14, 2025

Changes

Testing you performed

Please add any demo scripts to https://github.com/TomographicImaging/CIL-Demos/tree/main/misc

Related issues/links

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

@hrobarts hrobarts requested a review from a team as a code owner August 14, 2025 08:34
Copy link
Copy Markdown
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.

  • you'd have to add olefile to run_constrained
  • you'd have to add zenodo_get, olefile, dxchange, ipywidgets, etc to the README table's "optional deps" rows
  • you'd be violating conda's lack of first-class support of trivial optional deps
    • conda install pkg was designed to be equivalent to pip install pkg[all]
    • alternatively could have a separate package conda install cil-base (similar to matplotlib-base approach)

@hrobarts hrobarts closed this Aug 14, 2025
@casperdcl
Copy link
Copy Markdown
Member

reopening for future discussion

@gfardell
Copy link
Copy Markdown
Member

As long as these are in the environment files I think we're good to merge. Maybe in the table we can add CT data reader dependancies as a sub section as this is where most of our small packages come in and won't be universally needed or wanted.

@hrobarts hrobarts self-assigned this Nov 3, 2025
@hrobarts
Copy link
Copy Markdown
Contributor Author

hrobarts commented Nov 3, 2025

As long as these are in the environment files I think we're good to merge. Maybe in the table we can add CT data reader dependancies as a sub section as this is where most of our small packages come in and won't be universally needed or wanted.

Hi @gfardell, yes dxchange and olefile are in the cil_demos, cil_demos_cpu and cil_test environment files. zenodo_get isn't in them but I will add them now.

I've added olefile and dxchange to the dependency table in the README. As discussed I also moved the installation with the env files to the top of the installation instructions section and added a note on how to rename the environment and activate it.

@hrobarts hrobarts moved this from Todo to In Progress in CIL work Nov 5, 2025
@hrobarts hrobarts moved this from In Progress to Blocked in CIL work Nov 11, 2025
Signed-off-by: Hannah Robarts <77114597+hrobarts@users.noreply.github.com>
@hrobarts hrobarts merged commit 1cf8472 into master Nov 20, 2025
11 checks passed
@hrobarts hrobarts deleted the io_run_reqs branch November 20, 2025 09:56
@github-project-automation github-project-automation Bot moved this from Blocked to Done in CIL work Nov 20, 2025
casperdcl added a commit that referenced this pull request May 8, 2026
casperdcl added a commit that referenced this pull request May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants