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

Fix CI #348

Merged
merged 6 commits into from
Sep 25, 2024
Merged

Fix CI #348

merged 6 commits into from
Sep 25, 2024

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Sep 17, 2024

  • Add in the shim for macos.

Copy link

codecov bot commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.79%. Comparing base (0c73a97) to head (691d119).
Report is 105 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #348      +/-   ##
==========================================
- Coverage   98.94%   98.79%   -0.16%     
==========================================
  Files          36       36              
  Lines        1996     1988       -8     
==========================================
- Hits         1975     1964      -11     
- Misses         21       24       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -9,4 +9,4 @@ dependencies:
- sphinx
- openmm
- pip:
- git+https://github.com/OpenFreeEnergy/ofe-sphinx-theme@main
- git+https://github.com/OpenFreeEnergy/ofe-sphinx-theme@a45f3edd5bc3e973c1a01b577c71efa1b62a65d6
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a known issue upstream

Copy link
Member

Choose a reason for hiding this comment

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

Can you link to the issue? I'd like to learn what's going on here so we can follow up later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here we go: OpenFreeEnergy/ofe-sphinx-theme#5

It's a fixaeble packaging issue but I've not had time to look into it.

@IAlibay IAlibay changed the title [WIP] Fix CI Fix CI Sep 17, 2024
@@ -29,7 +29,8 @@ jobs:
cache-environment: true
cache-downloads: true
create-args: >-
python=3.9
python=3.10
rdkit=2023.09.5
Copy link
Member Author

Choose a reason for hiding this comment

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

I've never been able to work that one out.

Copy link
Member

Choose a reason for hiding this comment

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

@IAlibay can you elaborate? Work what out here?

Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason or another mypy has been complaining about a certain error with rdkit 2024+. Something about self being defined twice for a certain object.

Our approach has just been to pin to an older version of rdkit, but honestly we should try to fix it properly.

@dotsdl dotsdl self-requested a review September 25, 2024 05:55
Copy link
Member

@dotsdl dotsdl left a comment

Choose a reason for hiding this comment

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

@IAlibay some questions, but nothing blocking.

Do we know why some of the CI runs appear to be waiting endlessly?

@@ -29,7 +29,8 @@ jobs:
cache-environment: true
cache-downloads: true
create-args: >-
python=3.9
python=3.10
rdkit=2023.09.5
Copy link
Member

Choose a reason for hiding this comment

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

@IAlibay can you elaborate? Work what out here?

@@ -9,4 +9,4 @@ dependencies:
- sphinx
- openmm
- pip:
- git+https://github.com/OpenFreeEnergy/ofe-sphinx-theme@main
- git+https://github.com/OpenFreeEnergy/ofe-sphinx-theme@a45f3edd5bc3e973c1a01b577c71efa1b62a65d6
Copy link
Member

Choose a reason for hiding this comment

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

Can you link to the issue? I'd like to learn what's going on here so we can follow up later.

@IAlibay
Copy link
Member Author

IAlibay commented Sep 25, 2024

Do we know why some of the CI runs appear to be waiting endlessly?

It's because we've changed the matrix and some of the checks are enforced as "necessary" in the gufe repo settings, so github is waiting for runners that don't exist.

@IAlibay IAlibay merged commit 7d165a3 into main Sep 25, 2024
9 of 10 checks passed
@IAlibay IAlibay deleted the ci-fixing branch September 25, 2024 08:01
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