GilsonLabUCSD / pAPRika

Advanced toolkit for binding free energy calculations
BSD 3-Clause "New" or "Revised" License
30 stars 14 forks source link

Fixing versioneer issue #201

Closed jaketanderson closed 1 week ago

jaketanderson commented 1 week ago

The pAPRika package's versions were built with an old version of versioneer, which was giving me errors when running pip install ., so I included versioneer in the development conda environment. With versioneer 0.29 I ran versioneer init and versioneer install which regenerated the files paprika/__init__.py and paprika/_version.py. These changes solved the issue.

I also specified numpy<2 (which I think is currently redundant because pyMBAR requires that) and added jupyter and jupyter_contrib_nbextensions to the test environment, which allows one to run jupyter nbconvert <notebook.ipynb> --to script immediately after creating the environment; this is useful for development because one can quickly turn the tutorial notebooks into scripts then run them.

jaketanderson commented 1 week ago

I'd like CI to run on this PR. Can someone re-enable the CI workflow for the repository please?

slochower commented 1 week ago

I would avoid pinning numpy if you don't have to.

I think you have to add a reviewer to allow merging. I'm not sure how to enable CI on this repository now that it lives in the Gilson lab space.

j-wags commented 1 week ago

There was a "CI was disabled because of inactivity" message on this page, and I clicked an "enable" button to tell it to start again. Maybe push another commit here and see if it runs?

j-wags commented 1 week ago

Oh haha apparently I can push commits to your fork. Looks like CI is going now.

j-wags commented 1 week ago

Looks like codecov is being obnoxious now. I think a GilsonLabUCSD admin (I think the only one is @slochower) will need to do some configuration for codecov.

It's helpful to have continuous codecov reports to get alerted if a whole branch of testing falls offline, but since you're bringing CI back from the dead the old reports probably aren't accessible anyway. So you could probably just comment out the codecov part from the workflow yaml.

jaketanderson commented 1 week ago

Looks like codecov is being obnoxious now. I think a GilsonLabUCSD admin (I think the only one is @slochower) will need to do some configuration for codecov.

I think the codecov issue in the python 3.10 test is because I've made too many codecov requests in too short a time:

[2024-09-05T18:18:46.609Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 1349s.', code='throttled')}

Maybe if I make another cosmetic commit, we won't see the same rate limit issue. As for the python 3.9 test, unit tests are failing: ERROR paprika/tests/test_utils.py - TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'. I'm optimistic that this is straightforward to fix.

Edit:

The codecov upload still failed with the same error message. Maybe we should change to codecov/codecov-action@v4 here? It would need to be edited by someone with perms. https://github.com/GilsonLabUCSD/pAPRika/blob/6bbcb1909ed4e91746f0328fdb2aad64b3cbbf00/.github/workflows/ci.yaml#L68

slochower commented 1 week ago

Looks like codecov is being obnoxious now. I think a GilsonLabUCSD admin (I think the only one is @slochower) will need to do some configuration for codecov.

It's helpful to have continuous codecov reports to get alerted if a whole branch of testing falls offline, but since you're bringing CI back from the dead the old reports probably aren't accessible anyway. So you could probably just comment out the codecov part from the workflow yaml.

Mike and @jeff231li are also admins. I logged in to codecov -- it doesn't look like it has run recently but I'm not sure why.

image

Maybe you need to regenerate a new codecov token.

j-wags commented 1 week ago

Yeah, I think the token needs updating. It looks like codecov is limiting access for people connecting without a token (ie, using the v3 interface).

The OpenFF Toolkit codecov action uses v4 and defines the token as an environment variable, so that will need to be set as a pAPRika repository secret by an admin. https://github.com/openforcefield/openff-toolkit/blob/main/.github/workflows/CI.yml#L169-L174

It's easy for PRs to spiral in scope and become hard to coordinate on. So I recommend @jaketanderson NOT let codecov be a blocker to this PR, and instead comment out the codecov action from the ci.yaml file here entirely. It can be added back in a separate PR.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 46.97987% with 79 lines in your changes missing coverage. Please review.

Project coverage is 81.85%. Comparing base (17ae4e3) to head (affc777). Report is 10 commits behind head on master.

Additional details and impacted files
jaketanderson commented 1 week ago

It's easy for PRs to spiral in scope and become hard to coordinate on. So I recommend @jaketanderson NOT let codecov be a blocker to this PR, and instead comment out the codecov action from the ci.yaml file here entirely. It can be added back in a separate PR.

I updated codecov to v4 from v3. I also falsified "fail_ci_if_error" so if codecov has any more complaints, they will still appear in the workflow logs but won't cause the entire CI to fail. But with v4, codecov seems to have resolved its token issue.

At this point, the PR seems to fixed the versioneer issue and also gotten CI up and running (and passing for python 3.8 and 3.10). I think that's all I'll set out to do in this PR. After this one is merged I'll open up another one to fix failing tests for python 3.9.

j-wags commented 1 week ago

@slochower or @jeff231li - Jake can't merge until he has write access, could you update the repo settings to give him those?

jeff231li commented 1 week ago

@jaketanderson I have sent you an invite to the pAPRika repo and the GilsonLab organization.