Skip to content
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

Upgrading versions and adding Continuous Integration and Delivery #2

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

chris-zen
Copy link
Collaborator

@chris-zen chris-zen commented Aug 16, 2024

  • I upgraded the Python version requirement. Now it can work with Python versions 3.8 up to 3.11 (included), which has better support for modern tooling and wheels with pre-compiled binaries in PyPI. To be able to use 3.12 and above, we would require to upgrade at least bokeh.
  • I upgraded the requirements.txt enough so we could use wheels with pre-compiled binaries and I didn't break the tool when run.
  • I was not able to use modern build tools like rye, hatch or pdm because the support for cython and building a wheel with that is not good enough yet. Perhaps in the future.
  • As it was not possible to use modern build tools, I had to create a simple Makefile that provides a better/unified experience for development.
  • I created a workflow for CI/CD
  • It contains several jobs that will run on every push
    • runs checks and builds the Python distribution files
    • runs checks and builds the Docker image
    • build the documentation
  • It contains several jobs that will run only when a tag is created:
    • it will check that the version specified in the project and the tag are the same
    • it will publish the python distribution files to PyPI (although we would need credentials configured in the repo).
    • It will push the docker image into the DockerHub (I will also require credentials for that configured in the repo).
  • I updated the README to markdown and included updated instructions for the users to run OncodriveFML using the Docker image without having to install anything.

Thinks that might be addressed in other MRs:

  • add a check that runs the tool with docker end to end with some small datasets to make sure that all the dependencies integrate properly (given the dynamic nature of Python, getting a build doesn't guarantee that it will execute it, specially when there are no tests)

@chris-zen chris-zen force-pushed the upgrades-and-ci-cd branch 8 times, most recently from f38a101 to 7d25153 Compare August 16, 2024 23:38
@chris-zen chris-zen marked this pull request as ready for review August 17, 2024 00:10
@chris-zen chris-zen force-pushed the upgrades-and-ci-cd branch 4 times, most recently from 4a298f1 to e9efc61 Compare August 18, 2024 11:45
@FedericaBrando
Copy link
Member

Hi Christian! Thanks for the work, it looks great!

I have only one small comments about the documentation. Why do we need a job to build and publish docs? ReadTheDocs should trigger the build and publishing on their side when there is a new commit (or tag or whatever we want to use as a trigger). Is it a best practice to prevent and catch docs from failing?

BTW I have added DOCKER_PASSWORD and DOCKER_USERNAME for Docker as a organization secrets, the workflow should work and it should be able to push to docker.

@FedericaBrando FedericaBrando self-requested a review August 26, 2024 09:05
@chris-zen
Copy link
Collaborator Author

@FedericaBrando Thanks for the explanation on how RTD works. I've removed the job to publish docs now.

Copy link
Member

@FedericaBrando FedericaBrando left a comment

Choose a reason for hiding this comment

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

Thanks!

@chris-zen chris-zen merged commit a049bd7 into master Aug 26, 2024
6 checks passed
@chris-zen chris-zen deleted the upgrades-and-ci-cd branch August 26, 2024 18:51
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