Skip to content

Adds pyproject - #2117

Closed
purepani wants to merge 6 commits into
TomographicImaging:masterfrom
purepani:push-pykksptluszm
Closed

Adds pyproject#2117
purepani wants to merge 6 commits into
TomographicImaging:masterfrom
purepani:push-pykksptluszm

Conversation

@purepani

Copy link
Copy Markdown
Collaborator

Description

Addresses #2116
Taken(and added onto) from #2105.

Changes

Adds python dependencies to pyproject.toml, and updates CI to use these dependencies.

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

--->

@purepani
purepani force-pushed the push-pykksptluszm branch 24 times, most recently from dca30a8 to f4a9d8b Compare March 24, 2025 22:37
@purepani
purepani force-pushed the push-pykksptluszm branch from f4a9d8b to d11eb81 Compare March 24, 2025 22:42
@purepani
purepani force-pushed the push-pykksptluszm branch from d11eb81 to e297867 Compare March 24, 2025 22:46
@purepani
purepani force-pushed the push-pykksptluszm branch from e297867 to 14ada71 Compare March 24, 2025 22:58
@purepani
purepani force-pushed the push-pykksptluszm branch 2 times, most recently from d9b74dd to 5e59972 Compare March 24, 2025 23:22
@purepani

Copy link
Copy Markdown
Collaborator Author

I cannot figure out why the conda and docker builds are failing but otherwise this should be sufficient

@purepani
purepani requested a review from casperdcl March 24, 2025 23:37
@purepani purepani mentioned this pull request Mar 24, 2025
@gfardell

Copy link
Copy Markdown
Member

I cannot figure out why the conda and docker builds are failing but otherwise this should be sufficient

It looks like you're missing pywavlets (pywt) and h5py in your test environment.

It's also skipping a lot of tests as the optional (but required for out CI) packages aren't there:

----------------------------------------------------------------------
TEST SYSTEM CONFIGURATION
CIL version:  24.3.1.dev25+g5e59972b
{'has_astra': False,
 'has_ccpi_regularisation': True,
 'has_cvxpy': False,
 'has_ipp': True,
 'has_matplotlib': True,
 'has_numba': False,
 'has_nvidia': False,
 'has_tigre': True,
 'has_tomophantom': True}
----------------------------------------------------------------------

@purepani

purepani commented Mar 25, 2025

Copy link
Copy Markdown
Collaborator Author

I cannot figure out why the conda and docker builds are failing but otherwise this should be sufficient

It looks like you're missing pywavlets (pywt) and h5py in your test environment.

It's also skipping a lot of tests as the optional (but required for out CI) packages aren't there:

----------------------------------------------------------------------
TEST SYSTEM CONFIGURATION
CIL version:  24.3.1.dev25+g5e59972b
{'has_astra': False,
 'has_ccpi_regularisation': True,
 'has_cvxpy': False,
 'has_ipp': True,
 'has_matplotlib': True,
 'has_numba': False,
 'has_nvidia': False,
 'has_tigre': True,
 'has_tomophantom': True}
----------------------------------------------------------------------

Ok, I saw the missing packages pywt and h5py but didn't notice this, so I thought it was just those 2 packages which confused me(the though it's still a bit confusing as matplotlib still seems to be installed).
All of the packages should be installed and I'm not quite sure why they aren't. I'll look into this again later.

@purepani
purepani force-pushed the push-pykksptluszm branch 3 times, most recently from 1c8b8d2 to d6b8063 Compare March 25, 2025 17:56
@purepani

purepani commented Mar 25, 2025

Copy link
Copy Markdown
Collaborator Author

Ok, so I think I see the problem now.
Since the purpose of the conda CI is to work in an isolated conda environment, I can't just install stuff into the environment externally(both technically without hacks and conceptually).
conda doesn't seem to have the capability to work with pyproject.toml files(see conda/conda#12462), and, if I understand conda correctly, this dependency information also needs to be included in the recipe file.

A few options:

  1. duplicate dependencies in recipe files and pyproject. I don't like this solution, but it is a short term solution, and could potentially be done as an intermediate step. It'll be very annoying to deal with while it's an intermediate step, since dependencies will have to manually be updated in multiple places.
  2. Prepare the project so that it's publishable to pypi, publish to pypi, and then upload to conda-forge(as demonstrated here). To me this seems more logical, but does require a bit more work.
  3. Use an experimental feature in rattler-build to read the pyproject file.

I'm not sure how to do this super incrementally; the work that's been done so far is reasonably close to making 2. possible, but I would definitely prefer a more incremental strategy if anyone has suggestions.

@purepani

purepani commented Mar 25, 2025

Copy link
Copy Markdown
Collaborator Author

Nevermind: I think I found a way to get this to work with the tool grayskull. It can generate recipes from sdists, so I'll try that.

@purepani
purepani force-pushed the push-pykksptluszm branch from d6b8063 to 14ada71 Compare March 25, 2025 23:04
casperdcl added a commit that referenced this pull request Mar 26, 2025
- fixes #2116
  + fixes #1962 (related: #1964)
  + fixes #2074 (related: #2104)
- closes #2117
casperdcl added a commit that referenced this pull request Mar 26, 2025
- fixes #2116
  + fixes #1962 (related: #1964)
  + fixes #2074 (related: #2104)
- closes #2117
casperdcl added a commit that referenced this pull request Mar 26, 2025
- fixes #2116
  + fixes #1962 (related: #1964)
  + fixes #2074 (related: #2104)
- closes #2117
casperdcl added a commit that referenced this pull request Apr 1, 2025
- fixes #2116
  + fixes #2074 (related: #2104)
- closes #2117
- part of #1875
- related to #1961, #2090
casperdcl added a commit that referenced this pull request Apr 1, 2025
- fixes #2116
  + fixes #2074 (related: #2104)
- closes #2117
- part of #1875
- related to #1961, #2090
@casperdcl casperdcl closed this in e2878f3 Apr 17, 2025
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