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

Open
wants to merge 56 commits into
base: develop
Choose a base branch
from

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

Getting back to this PR, which has expanded beyond just pyproject.toml/Docker/CI matters to include analysis code changes to cosmo_val.py..

@sfarrens I tried to address all of your comments from last time and I'm pretty happy with the non-cosmo_val.py changes, so when you're back from vacation I'd welcome a second review!

@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]>
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