-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add pyproject.toml with requires=["numpy"] #141
Comments
I think it needs all of |
This simple |
Seem Mac+Linux build is failing with this. |
Found the logs: https://app.travis-ci.com/github/jonwright/ImageD11/jobs/560505555 seem to be failing on Xenial Linux with:
was used to pre-install |
I didn't get time to look in detail - no idea what goes wrong. Packaging has never worked for compiled code. |
Closing this and keeping the rollback as the current solution for now. |
Hi all, I'm running into problems with this now - I'm trying to set up # [...]
[tool.poetry.dependencies]
python = ">=3.8,<3.12"
ImageD11 = {git = "https://github.com/FABLE-3DXRD/ImageD11.git"}
# [...] This installs fine with
Any ideas? If I were using |
@jonwright @AxelHenningsson I believe I may have a solution here (albeit not a very good one) - the numpy error about mismatched versions is I think due to the following scenario:
I made a fork here - if you force the same numpy version in build and runtime dependencies, it solves the problem for me. The only issue then is that ImageD11 is tied to a specific numpy version, which causes issues with minimum python versions etc... a good excuse to drop Python 2 support? :) |
Nice that someone is bravely still trying to solve this. Did the build test on linux and max and the pytests pass with this? Cheers |
I thought the pep517 thing was about editable installs? Pinning the numpy version is not a good solution for the day you get another package that pins a different version. What is the actual problem with your poetry thing? Does it give any logs that say what it did? Can you import ImageD11 and print From what I can see, it works on another computer here at ESRF (below). In the longer term, it might be useful to move the C code over to some other build system.
|
I'm pretty embarrassed now - after working on this all day yesterday I now can't reliably recreate the error I was seeing with a minimal example.
But now whatever I try, I can't get a simple Poetry project set up that fails reliably. It very well might have been due to some dodgy virtual environment stuff on my machine. |
Problems recreating anything is the essence of python packaging 😄 I guess we could change that error message to print some more useful debugging information (cImageD11.file and sys,path perhaps). The numpy versioning thing might go wrong if packages are added one by one, and a later package 'upgrades' or 'downgrades' something used by a previous one. It used to come up with conda when you start out with the fast intel/mkl version of numpy and then lose it when installing some package. |
@jonwright Exactly - could we write a file during the build process that contains information about the version of numpy used to build ImageD11 (and its path, maybe)? This could help troubleshoot these issues. I'm pretty sure the issue arises when whatever packaging dependency solver you're using detects that there's a version of numpy already installs, builds ImageD11 with that, then updates it when the rest of the packages are installed. |
Seems I suppressed the pyproject.toml in commit #96dada3, but I don't remember why. Putting it back and waiting for the CI... |
It should be back #161, so I close the issue. |
As posted on stack there is perhaps a simple fix to make
pip install
work even if numpy is required insetup.py
to compile?Could perhaps be nice to try and add a
pyproject.toml
withrequires = [ "numpy"]
and see if the installation issue can be fixed. This is mostly annoying if anyone wants to depend onImageD11
, since they are then required to propagate the "please install numpy first documentation" into their own docs.Best,
Axel
The text was updated successfully, but these errors were encountered: