-
Notifications
You must be signed in to change notification settings - Fork 2
Fix: add gsl install to macos14 to pass CI. #24
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
base: main
Are you sure you want to change the base?
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
sbillinge
left a comment
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.
Is it better to do this or to add gsl to the tests.txt requirements (or the conda requirements)?
|
Yes, it might be a better way. |
sbillinge
left a comment
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.
We didn't need it in both places. Just tests unless it is needed to run the code itself, in which case, just in conda.txt and pip.txt
|
@stevenhua0320 I looked more into this and it will be a runtime dependency, so we need to add it to the Actually, looking at diffpy.pdffit2 it seems that this was addressed (and tests are passing there) in diffpy/diffpy.pdffit2#144 . There gsl was added to tl;dr: Let's try a fix where we see if adding gsl as a dependency to the |
|
@sbillinge I have checked the feedstock in |
Something is not right here. Gsl is not a requirement of distanceprinter but it is a requirement of pdffit2. It would be good to get to the bottom of why it is not getting loaded as part of the pdffit2 requirements |
|
It is because in macos14 the homebrew removed many bottles, including the one for |
if gsl is a dependency of pdffit2, then it should be installed already. This is my point. |
|
I'm afraid it is not defined as a dependency but as build requirement in |
|
Let's try adding it to run and we will see if it allows the tests to pass for distanceprinter |
|
I think I have added in the feedstock in the |
@sbillinge ready for review, we should test this whether it would be successful to build CI on macos14.