-
Notifications
You must be signed in to change notification settings - Fork 23
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 install with latest desiutil without DesiTest #832
Conversation
Ugh, tests are failing while installing cfitsio. Will need some more work to restore that. |
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.
This looks fine to me. It looks like you may need to sort out dependencies for compiling fitsio
. The first thing to try is using ubuntu-22.04
instead of ubuntu-latest
.
Well that was unsatisfying. matplotlib is trying to force an upgrade to numpy>2, which isn't compatible with the other packages. Using --no-deps for matplotlib means it doesn't have other dependencies like PIL, which I haven't figured out how to install yet given the other package constraints... |
I went through something very similar last week. I'll point you to it in the morning. |
You might want to compare with desihub/desimodel#178. |
Finally, success. In the end the coverage and regular unit tests have a somewhat different matrix of dependency versions because that is just how it landed, but I'll declare that to be a feature for providing some examples of combinations of versions that work. |
desihub/desiutil#212 removed
desiutil.setup.DesiTest
, which broke the ability to install desitarget when using the latest desiutil. This PR fixes that by removing the DesiTest import from setup.py. Now to run desitarget tests you have to usepytest
and notpython setup.py test
. That has been our recommendation for awhile (and how github actions and NERSC tests have been performed), but this PR removes support for the oldpython setup.py test
option.With current main this fails with "ImportError: cannot import name 'DesiTest' from 'desiutil.setup'":
Replacing that last line with this branch works
After merging this branch, it should be possible to install desitarget/main when using desiutil/main.