conda-forge / petsc4py-feedstock

A conda-smithy repository for petsc4py.
BSD 3-Clause "New" or "Revised" License
0 stars 15 forks source link

petsc4py setup.py injects stricter dependency range than numpy ABI #104

Closed minrk closed 1 month ago

minrk commented 1 month ago

numpy 2.0 builds are ABI compatible with numpy 1.19. But petsc4py setup.py injects numpy >=2,<3 in metadata, causing downstream pip check to fail with compatible numpy 1.19.

Two options:

conda-forge-webservices[bot] commented 1 month ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

dalcinl commented 1 month ago

@minrk I could patch setup.py upstream, but I think I need a clarification about the exact change.

  1. If petsc4py is built with numpy==2.*, then the proper pin is numpy>=1.19,<3.
  2. If petsc4py is built with numpy==1.*, then the proper pin is numpy<2 .

Are both (1) and (2) correct? I think petsc4py source tarball has to still support building with an old NumPy 1.* release.

EDIT: I'm leaning to the following in petsc4py's setup.py:

major = int(numpy.__version__.partition('.')[0])
numpy_pin = 'numpy>=1.19' if major >= 2 else 'numpy<2'
minrk commented 1 month ago

Yes, both of those are correct. Ref: numpy's docs.

You can also govern the ABI target at build time by defining NPY_TARGET_VERSION during compilation, if you want to set an explicit (e.g. higher) baseline.

dalcinl commented 1 month ago

Upstream merge request: https://gitlab.com/petsc/petsc/-/merge_requests/7745 patch: https://gitlab.com/petsc/petsc/-/commit/f566d94923bc9793725926f8cb3f9d964c583ae2.patch

dalcinl commented 1 month ago

@minrk Should we just update your commit to add the upstream patch and remove the pin in meta.yml?

minrk commented 1 month ago

Yeah, that sounds good

dalcinl commented 1 month ago

@minrk Is all good from your side? Should we merge?