Skip to content

Pyproject.toml-based installation, updated dependencies, and automatic Docker builds #144

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

Merged
merged 70 commits into from
Jul 17, 2025

Conversation

cailmdaley
Copy link
Collaborator

The shapepipe and sp_validation packages have inconsistent version requirements, causing dependency versions to change when installing both packages in the same environment. This PR attempts to make the dependencies of the two packages consistent (and adds a few new dependencies).

While I was at it, I switched form the old setup.py installation method to the modern pyproject.toml approach.

I also plan to set up automatic GitHub builds of a docker container for the package, building on top of the container for the shapepipe package.

@cailmdaley cailmdaley marked this pull request as draft March 6, 2025 16:48
@cailmdaley cailmdaley self-assigned this Mar 6, 2025
@cailmdaley cailmdaley requested a review from sfarrens March 6, 2025 16:48
Copy link
Member

@sfarrens sfarrens left a comment

Choose a reason for hiding this comment

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

Hey @cailmdaley, looking good so far. 👏 I added a few comments on things you can look into. One general point to note is that we probably won't want/need any pinned dependencies in the pyproject.toml as we can tag versions of the Docker image instead.

@cailmdaley cailmdaley marked this pull request as ready for review April 8, 2025 01:38
@cailmdaley
Copy link
Collaborator Author

thank you for the helpful comments Sam. to your general point, wouldn't it be better to specify python dependencies in the pyproject.toml since those dependencies should apply outside the container as well?

@cailmdaley
Copy link
Collaborator Author

I think this is ready to be merged pending review, I will do one more run through the whole script to make sure everything works.

Co-authored-by: Martin Kilbinger <[email protected]>
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@sfarrens
Copy link
Member

sfarrens commented Jul 4, 2025

Hi @cailmdaley, my apologies for being so slow to finalise this review. I have run the following tests:

  1. pyproject.toml
  • Build on CC-IN2P3 ✅ - build succeeds but I had to mess with the versions of scipy and statsmodels to import the package
  • Build on macOS ❌ - build fails
  1. Dockerfile
  • Apptainer build on CC-IN2P3 ✅ - works out of the box
  • Local Docker build on macOS ❓- it has not crashed but the build has been running for a very long time, certainly too long for real world users
  • Docker pull of pre-built image from ghcr ✅ - no problem pulling, however I would argue that the resulting image is basically unusable because of how slow it runs on Apple Silicon
  • I will try one final local build test on my Ubuntu machine tonight just for comparison

In summary, if the scope of this PR is simply to add a pyproject.toml setup and a Dockerfile that works only for what it absolutely has to work on, then I would say go ahead and merge this PR but just keep in mind that some future PRs will be needed to opened to make this a more usable tool for the community.

@cailmdaley
Copy link
Collaborator Author

Thank you Sam! I really appreciate you taking the time to check this. It's good to hear that the Docker build works on another system and a plain pip installation almost works out of the box.

Yes I think making things work for MacOS is beyond the scope of this PR. Making it usable more widely would be nice longer-term, but for now I think it's most important to get these changes merged so we can use them for the ongoing UNIONS cosmic shear analyses. I will go ahead and merge.

@cailmdaley cailmdaley merged commit 5510ca1 into develop Jul 17, 2025
1 check passed
@cailmdaley cailmdaley deleted the pyproject_docker branch July 17, 2025 15:43
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.

4 participants