-
Notifications
You must be signed in to change notification settings - Fork 297
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
fixing ci pipeline for python3.9 and python3.13 #435
base: master
Are you sure you want to change the base?
Conversation
a29d066
to
1558afc
Compare
Hey @JarrettSJohnson ! |
80a4b56
to
5a67203
Compare
4a178d5
to
979cc02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Thanks for the major effort here; I've never been a fan of trying to wrangle conda with CI.
The only major feedback I have for this is the mixing of conda-forge and PyPI packages--which I remember is generally not recommended. However, if it's for CI purposes, I don't really care too much, but I don't feel comfortable suggesting that to our users. Ideally I would like conda-forge to recognize pyproject.toml files: https://github.com/conda/conda/issues/12462
. This seems possible with pixi
, and I've been thinking about making a switch to it.
run: | | ||
conda deactivate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this deactivate if you're just activating another environment the next line?
@@ -14,21 +14,26 @@ authors = [ | |||
{name = "Schrodinger", email = "[email protected]"}, | |||
] | |||
dependencies = [ | |||
"biopython>=1.80", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why this is added here? Biopython was added to CI for testing purposes (for optional features), but is not a requirement for PyMOL.
"numpy>=2.0", | ||
] | ||
|
||
[build-system] | ||
build-backend = "backend" | ||
backend-path = ["_custom_build"] | ||
requires = [ | ||
"cmake>=3.30.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cmake>=3.13 is sufficient since that is what's required in the CMakeLists.txt
libpng ` | ||
libxml2 ` | ||
libnetcdf ` | ||
libffi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although you linked the error and solution in the description. I'm still a bit unsure where this is coming from. I'm fairly confident this spawned from now not using conda-forge's Python
python=${{ matrix.python-version }} ` | ||
pip ` | ||
catch2=2.13.3 ` | ||
cxx-compiler ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably correct, but what caused this to be needed now?
For more information on the solution, check this link.
This module is quite old, and I have not had to work with it before. I would appreciate any assistance with testing it.
Additionally, the use of cgi can still be found in examples