Skip to content

ci: fix failing tests and build related to pKa-ANI dependencies#443

Merged
kiyoon merged 10 commits into
Electrostatics:masterfrom
adnaksskanda:pka-ani_py310
Dec 7, 2025
Merged

ci: fix failing tests and build related to pKa-ANI dependencies#443
kiyoon merged 10 commits into
Electrostatics:masterfrom
adnaksskanda:pka-ani_py310

Conversation

@adnaksskanda
Copy link
Copy Markdown
Contributor

Made an edit on @kiyoon's recommendation to get pdb2pqr[pkaani] version with cpu torch to install, hopefully now this build pipeline will work

@kiyoon
Copy link
Copy Markdown
Collaborator

kiyoon commented Nov 13, 2025

  1. To use uv, I recommend following this: https://docs.astral.sh/uv/guides/integration/github/. Since you did not do this, it's required to create and activate an environment each step. But here's a better way:
# Put this after setup-python

- name: Install uv and activate environment
  uses: astral-sh/setup-uv@v7
  with:
    # Use uv venv to activate a venv ready to be used by later steps
    activate-environment: "true"
  1. no need to upgrade pip anymore because we're using uv now.
  2. actions/checkout@v4 -> let's upgrade to v5

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

@kiyoon I see, so sorry did not read the astral guide when you shared it the first time. I have updated the python-package.yml file to follow what you have provided

@kiyoon
Copy link
Copy Markdown
Collaborator

kiyoon commented Nov 13, 2025

I think the environment problem is gone, but there's a problem with the test that the files not being able to be fetched. I don't know what happened

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

adnaksskanda commented Nov 20, 2025

@kiyoon I think there were two issues - one was that in pkaani_test.py the PKAANI_TEST_DIR was set to Path("pkaani_tests") instead of Path("tests/pkaani_tests"). The other was some dependency issues in pkaani that I fixed - can we try to rerun it through the build pipeline one more time?

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

adnaksskanda commented Nov 20, 2025

Is there a way to get it to freshly download the dependencies? I updated pkaani on pyPI but it seems to still be showing the old version in the build pipeline installation: + pkaani==0.1.10. If we can get the build pipeline installation to not rely on its cache and redownload the newest package version (pkaani==0.1.13), it should pass the tests

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

@kiyoon added the --no-cache option to uv so it pulls the latest pyPI version of pkaani, should work this time (fingers crossed)

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

@kiyoon still defaulting to pkaani==0.1.10 the old version in the build pipeline. I added --refresh and --upgrade to the uv command so now hopefully it installs the correct version of pkaani in the build pipeline.

@kiyoon
Copy link
Copy Markdown
Collaborator

kiyoon commented Nov 20, 2025

It's probably due to the dependency issue. If you run uv pip compile pyproject.toml --extra pkaani then it shows the old version of pkaani. Thus you need to fix this first. Maybe the newer version of pkaani conflicts with some package like numpy. Because of that and other dependencies, the uv/pip decides that 0.1.13 is not compatible with the current pdb2pqr maybe.

EDIT: About the output below

When you try to install pkanni in the environment with pdb2pqr without it, it forces to install nunpy v1. So maybe I think that could be the culprit. In any case, we need to avoid numpy v1 for future compatibility because there are already many packages that don't support it

$ uv pip install pkaani
Using Python 3.10.16 environment at: /Users/kiyoon/bin/miniforge3/envs/pdb2pqr
Resolved 34 packages in 282ms
Prepared 15 packages in 10.54s
Uninstalled 1 package in 93ms
Installed 22 packages in 328ms
 + ase==3.26.0
 + contourpy==1.3.2
 + cycler==0.12.1
 + filelock==3.20.0
 + fonttools==4.60.1
 + fsspec==2025.10.0
 + joblib==1.5.2
 + kiwisolver==1.4.9
 + lark-parser==0.12.0
 + matplotlib==3.10.7
 + mpmath==1.3.0
 + networkx==3.4.2
 - numpy==2.2.3
 + numpy==1.26.4
 + pillow==12.0.0
 + pkaani==0.1.13
 + pyparsing==3.2.5
 + scikit-learn==1.6.1
 + scipy==1.15.3
 + sympy==1.14.0
 + threadpoolctl==3.6.0
 + torch==2.9.1
 + torchani==2.2

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

adnaksskanda commented Nov 21, 2025

@kiyoon Thank you for the suggestion, you were totally right. I removed the numpy<2 pin from the pyPI pkaani, it is not needed for the pkaani import and usage in pdb2pqr. Can we try again? I think now this should work - at least it worked when I tested it locally just now.

When the modules load now uv should be loading pkaani==0.1.14, which should be compatible with everything.

@kiyoon
Copy link
Copy Markdown
Collaborator

kiyoon commented Nov 21, 2025

if it works, you may remove --upgrade --refresh --no-cache so it doesn't confuse future contributors

@adnaksskanda
Copy link
Copy Markdown
Contributor Author

@kiyoon it worked finally! took out those options from the uv pip command.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses linting issues in the pKa-ANI integration tests and modernizes the CI/CD pipeline to use uv for faster, more reliable dependency management with CPU-only PyTorch installation.

  • Reformatted test parametrization decorators to follow proper Python style guidelines
  • Fixed relative path for pKa-ANI test data directory
  • Updated GitHub Actions workflow to use uv package manager with CPU-only PyTorch backend
  • Upgraded GitHub Actions to their latest versions (checkout@v5, upload-artifact@v5)

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/pkaani_test.py Corrected test data directory path and reformatted pytest parametrize decorators for better readability
tests/common.py Removed extraneous blank lines for cleaner code formatting
.github/workflows/python-package.yml Modernized CI pipeline by integrating uv package manager, updated action versions, and configured CPU-only PyTorch installation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kiyoon kiyoon changed the title linted pKa-ANI integration, made a small edit to python-package.yml actions ci: fix failing tests and build related to pKa-ANI dependencies Dec 7, 2025
@kiyoon kiyoon merged commit cbbffd0 into Electrostatics:master Dec 7, 2025
13 checks passed
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.

5 participants