SINTEF / Splipy

Spline modelling made easy.
GNU General Public License v3.0
100 stars 18 forks source link

Splipy requires Numpy 1.20, conflict with conda-forge packages #157

Closed ghost closed 2 years ago

ghost commented 2 years ago

I'm afraid that this commit changing the minimum numpy version to 1.20 in setup.py

https://github.com/SINTEF/Splipy/blob/master/setup.py#L26

may be causing this package to be incompatible with many other packages. I've been maintaining a package for splipy on conda-forge (I'd be happy to turn this over to someone else!) and I've run into some problems with the 1.5.8 release. Doing conda install -c conda-forge splipy causes conda to report Found conflicts! Looking for incompatible packages and then spends hours checking.

Is there anything in Splipy which requires features only present in Numpy 1.20? It would be best to have the minimum version requirement as low as possible, for cross-compatibility with other packages and installations.

TheBB commented 2 years ago

When I tried Numpy 1.20 for the first time I got binary compatibility errors when running on Python installations with older versions of Numpy.

https://numpy.org/devdocs/release/1.20.0-notes.html#c-api-changes

Since Splipy contains compiled components that use the binary Numpy API, and, as far as I know, I can't ship two different versions on PyPI depending on Numpy version, I have to choose to ship either one that is compiled against Numpy >= 1.20 or Numpy < 1.20.

TheBB commented 2 years ago

To be clear, if built against Numpy < 1.20, Splipy should work fine. But if built against Numpy >= 1.20 and used on an older version, this happens:

In [1]: import splipy
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-1-388c9eb1f08c> in <module>
----> 1 import splipy

~/.local/lib/python3.8/site-packages/splipy/__init__.py in <module>
----> 1 from .basis import BSplineBasis
      2 from .splineobject import SplineObject
      3 from .curve import Curve
      4 from .surface import Surface
      5 from .volume import Volume

~/.local/lib/python3.8/site-packages/splipy/basis.py in <module>
      8 
      9 from .utils import ensure_listlike
---> 10 from . import basis_eval, state
     11 
     12 __all__ = ['BSplineBasis']

splipy/basis_eval.pyx in init splipy.basis_eval()

ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject
ghost commented 2 years ago

Hi - I thought that commenting on #152 would automatically reopen it, and when it didn't I opened a new issue (this one). Sorry for the duplicate.

You are maintaining the PyPI package and have to make the appropriate choices there. My project is using conda-forge and has different constraints. The change of binary ABI for numpy seems to be a bit of a pain-point. I don't know how PyPI is managing this - I imagine it would create a lot of inter-package conflicts. But if you're not having this problem, then the setup.py is fine for PyPI and doesn't need to be changed.

In conda-forge the numpy dependency is managed like this:

https://conda-forge.org/docs/maintainer/knowledge_base.html#linking-numpy

https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/master/recipe/conda_build_config.yaml#L641

that is, they "pin" the numpy version to the oldest numpy version possible (1.18 or 1.19 depending on architecture)

In conda-forge I have the ability to apply patches to the setup.py, so I can change it to suit my needs. As long as you are maintaing the PyPI package and I am maintaining the conda-forge package, this approach should be workable. If you're interested in taking over the packaging for conda/conda-forge, I'll hand over the reins.

Note that I volunteered to maintain the conda-forge package because we are using Splipy as part of a large fluid dynamics suite and every other component we use is coming from conda-forge - so having Splipy in conda-forge is a convenience for our users to be able to install everything they need to run our software from a single source. I don't want to cause any extra difficulty for you by doing this - your software is helpful to us and our users.

Thanks for your attention and helpful comments, I think we can close this now.

TheBB commented 2 years ago

Sounds good to me, you're of course welcome to patch the setup file as you like. Happy to hear Splipy is useful to you.