Skip to content
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

Build packages compatible with multiple NumPy version #32

Merged
merged 2 commits into from
Jul 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,9 @@ osx_image: xcode6.4
env:
matrix:

- CONDA_NPY=111 CONDA_PY=27
- CONDA_NPY=112 CONDA_PY=27
- CONDA_NPY=113 CONDA_PY=27
- CONDA_NPY=111 CONDA_PY=35
- CONDA_NPY=112 CONDA_PY=35
- CONDA_NPY=113 CONDA_PY=35
- CONDA_NPY=111 CONDA_PY=36
- CONDA_NPY=112 CONDA_PY=36
- CONDA_NPY=113 CONDA_PY=36
- CONDA_PY=27
- CONDA_PY=35
- CONDA_PY=36
global:
# The BINSTAR_TOKEN secure variable. This is defined canonically in conda-forge.yml.
- secure: "azJjQJJ3AwRWcueVlo7EyYPB5ls04Vx8IkYVIfLoRc84NJ1aePU2B15gBaHROPx9jrDHcVzfWLKaq+qj19C1kMUViIgxeMJ/+2kst8d2TrPoVy9fiHRWUnim/MNluUhBfnXxyeLxnWbJX4pBJBOpAiU1wi+WO/d88EWxfx2aPhQfiEXuD+j66mGYmOVrZ2mwPDVKFOp+8r9bmT9OE162HKJEepVcWiNi+wR58rp91wV3rA18Ouwzw1AmuQb88l3bdKjN8h2pAbIKOopDTv4pU89hLd3HqPkyAuTF67dSQzliMhZBjpl9C0468GH+rs7/EUUqoZIVMNRpZ4j6clqwDf/GB49bUnUZlbLavkqkbyK4NdLIX2zu9iWWQkpXVj0VZDCyFBsRqjOcElM6nO7NhRfBMVrdUgzVFLID24G58u/qMKORpX6d17t5e9baNRBDhe/3cRokB1et681g8iFKRmVdhJ+wCvbMx6KjEVP6csAV46713PoNGv9GUx4utJyLx/ttlxrGLMFcix6mpZV3kBRfk2prySzuZ3DvVqIRpujiPK+3XvIUPXIfxmsFmf9nxPJ05GQ4FcLBC81wKmbd+icCrPVwGZyUaHqGfNaBM06av/6FcAa+8muMiz8CnLJb3d7rknJKrexXTAYdCPH7mHLHDFUqE6+gfSY222jP1zs="
Expand Down
66 changes: 0 additions & 66 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,92 +10,26 @@ environment:

matrix:
- TARGET_ARCH: x86
CONDA_NPY: 111
CONDA_PY: 27
CONDA_INSTALL_LOCN: C:\\Miniconda

- TARGET_ARCH: x64
CONDA_NPY: 111
CONDA_PY: 27
CONDA_INSTALL_LOCN: C:\\Miniconda-x64

- TARGET_ARCH: x86
CONDA_NPY: 112
CONDA_PY: 27
CONDA_INSTALL_LOCN: C:\\Miniconda

- TARGET_ARCH: x64
CONDA_NPY: 112
CONDA_PY: 27
CONDA_INSTALL_LOCN: C:\\Miniconda-x64

- TARGET_ARCH: x86
CONDA_NPY: 113
CONDA_PY: 27
CONDA_INSTALL_LOCN: C:\\Miniconda

- TARGET_ARCH: x64
CONDA_NPY: 113
CONDA_PY: 27
CONDA_INSTALL_LOCN: C:\\Miniconda-x64

- TARGET_ARCH: x86
CONDA_NPY: 111
CONDA_PY: 35
CONDA_INSTALL_LOCN: C:\\Miniconda35

- TARGET_ARCH: x64
CONDA_NPY: 111
CONDA_PY: 35
CONDA_INSTALL_LOCN: C:\\Miniconda35-x64

- TARGET_ARCH: x86
CONDA_NPY: 112
CONDA_PY: 35
CONDA_INSTALL_LOCN: C:\\Miniconda35

- TARGET_ARCH: x64
CONDA_NPY: 112
CONDA_PY: 35
CONDA_INSTALL_LOCN: C:\\Miniconda35-x64

- TARGET_ARCH: x86
CONDA_NPY: 113
CONDA_PY: 35
CONDA_INSTALL_LOCN: C:\\Miniconda35

- TARGET_ARCH: x64
CONDA_NPY: 113
CONDA_PY: 35
CONDA_INSTALL_LOCN: C:\\Miniconda35-x64

- TARGET_ARCH: x86
CONDA_NPY: 111
CONDA_PY: 36
CONDA_INSTALL_LOCN: C:\\Miniconda36

- TARGET_ARCH: x64
CONDA_NPY: 111
CONDA_PY: 36
CONDA_INSTALL_LOCN: C:\\Miniconda36-x64

- TARGET_ARCH: x86
CONDA_NPY: 112
CONDA_PY: 36
CONDA_INSTALL_LOCN: C:\\Miniconda36

- TARGET_ARCH: x64
CONDA_NPY: 112
CONDA_PY: 36
CONDA_INSTALL_LOCN: C:\\Miniconda36-x64

- TARGET_ARCH: x86
CONDA_NPY: 113
CONDA_PY: 36
CONDA_INSTALL_LOCN: C:\\Miniconda36

- TARGET_ARCH: x64
CONDA_NPY: 113
CONDA_PY: 36
CONDA_INSTALL_LOCN: C:\\Miniconda36-x64

Expand Down
47 changes: 1 addition & 46 deletions ci_support/run_docker_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,65 +57,20 @@ conda clean --lock
conda install --yes --quiet conda-forge-build-setup
source run_conda_forge_build_setup

# Embarking on 9 case(s).
# Embarking on 3 case(s).
set -x
export CONDA_NPY=111
export CONDA_PY=27
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=112
export CONDA_PY=27
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=113
export CONDA_PY=27
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=111
export CONDA_PY=35
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=112
export CONDA_PY=35
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=113
export CONDA_PY=35
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=111
export CONDA_PY=36
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=112
export CONDA_PY=36
set +x
conda build /recipe_root --quiet || exit 1
upload_or_check_non_existence /recipe_root conda-forge --channel=main || exit 1

set -x
export CONDA_NPY=113
export CONDA_PY=36
set +x
conda build /recipe_root --quiet || exit 1
Expand Down
10 changes: 7 additions & 3 deletions recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,22 @@ source:
sha256: a777e07633d83d546c55706420179551c8e01075b53c497dcf8ae4036766bc66

build:
number: 0
number: 1
script: python setup.py install --single-version-externally-managed --record=record.txt

requirements:
build:
- python
- setuptools
- cython
- numpy x.x
- numpy 1.7.* # [py27]
- numpy 1.9.* # [py35]
- numpy 1.11.* # [py36]
run:
- python
- numpy x.x
- numpy >=1.7 # [py27]
- numpy >=1.9 # [py35]
- numpy >=1.11 # [py36]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have different NumPy versions for different Python versions? What would be wrong with requiring numpy >=1.7 for all Python versions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably because numpy 1.7 does not support py35 or py36

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, though admittedly there won't be a numpy 1.7 build for python 3.5 or 3.6 then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if there ever was a numpy 1.7 for python 3.5 or 3.6 the package built using this recipe would NOT work with them because numpy does not guarantee forward compatibility.

These lines properly describe the numpy requirement for the various Python versions. I don't think depending on the repository not having certain versions of a package is something we want to depend upon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the impression by looking at the release notes that not much has changed in the C API since NumPy 1.7 or at least what has changed would not present an issue when building. Though there may be issues at runtime for exceptionally old code.

Copy link
Member

@jakirkham jakirkham Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that does actually concern me a bit with this strategy is that if we have BLAS dependent packages that use NumPy/SciPy as interface to the BLAS, reaching back to such old versions will give us packages that are not compatible with how BLAS is used in conda-forge currently and thus mostly like the wrong BLAS. Given this, would it make more sense to restrict everything to say NumPy 1.10 or 1.11 when NumPy was added to conda-forge with BLAS support?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to restrict everything to 1.10 or 1.11. We've dropped numpy <= 1.10 in the old approach anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building against a "newish" numpy does not create a package which works with older numpy versions. It is necessary to specify the run requirement numpy be strictly greater than or equal to the build version. For example, installing the pandas from defaults built against numpy 1.12 and then force installing numpy 1.9 results in the following import time error:

$ python -c "import pandas"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/jhelmus/anaconda/envs/test/lib/python2.7/site-packages/pandas/__init__.py", line 26, in <module>
    from pandas._libs import (hashtable as _hashtable,
  File "/Users/jhelmus/anaconda/envs/test/lib/python2.7/site-packages/pandas/_libs/__init__.py", line 3, in <module>
    from .tslib import iNaT, NaT, Timestamp, Timedelta, OutOfBoundsDatetime
  File "pandas/_libs/src/numpy.pxd", line 157, in init pandas._libs.tslib (pandas/_libs/tslib.c:117317)
ValueError: numpy.dtype has the wrong size, try recompiling. Expected 88, got 96

Although conda-forge does not provide numpy version less than 1.10 they are available in defaults and I believe should be supported. I am willing to submit PRs to get conda-forge version of these older numpy version if that is desired.

As for the BLAS issue, any packages which links directly to BLAS/LAPACK should be pinning their BLAS implementation and versions. I suspect there are packages in the channel which do not do this correctly, these should be updated and the un-pinned packages removed. This BLAS pinning limits the the supported versions of numpy available to the package via the solver. I do not think this limitation should be introduced by overloading the numpy package to include a BLAS version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building against a "newish" numpy does not create a package which works with older numpy versions. It is necessary to specify the run requirement numpy be strictly greater than or equal to the build version.

I understand. However this is not I was proposing either.

Am proposing we pick 1.10 or 1.11 and build with that version and make it the lower bound as well across all Pythons. It will be the easiest to do for core, for reviewers, for maintainers and the most consistent for a variety of reasons already outlined. I don't really see any point to going to any more work than that as I would expect nearly all conda-forge users are on NumPy 1.10 or greater at this point.

Although conda-forge does not provide numpy version less than 1.10 they are available in defaults and I believe should be supported. I am willing to submit PRs to get conda-forge version of these older numpy version if that is desired.

That said, if you are willing to do the work to make this happen, no real opposition from me. Just seems like the average maintainer already has enough to keep in our heads (e.g. building on Windows using python as a proxy for VS, pinnings, etc.), it would be nice to not give them something new NumPy strategy that is more work than what they already do. Though again this is a concern IMHO not a blocker.

This BLAS pinning limits the the supported versions of numpy available to the package via the solver. I do not think this limitation should be introduced by overloading the numpy package to include a BLAS version.

I think the point is missed here. If someone pins BLAS as we tell them to do and pins NumPy as we tell them to do and these conflict, the package won't build. I'm almost certain we will hear grumbles about this. Hence not having old enough NumPy's built with the currently BLAS schema (and pinning) does present a blocker for the strategy outlined in this PR. How the knobs of NumPy version and BLAS version(s) are tuned will make this easier or harder to maintain in the long run. Adding Python in with these knobs complicates things. So I would urge that we all reach some agreement on where everyone feels comfortable tuning such knobs and how variable they can be.

At this point, we probably should roll this over into a webpage issue so that this doesn't get buried as it will be some sort of policy decision.

- python-dateutil
- pytz

Expand Down