conda-forge / prismatic_split-feedstock

A conda-smithy repository for prismatic_split.
BSD 3-Clause "New" or "Revised" License
3 stars 5 forks source link

Add python version #3

Closed ericpre closed 4 years ago

ericpre commented 4 years ago

Checklist

Tentative of alternative to https://github.com/conda-forge/prismatic_split-feedstock/pull/2. Even if I am not sure if pyprismatic needs python version specific build, in practise we can't make the recipe noarch, so we do need packages for python 3.6 and 3.7 to be able to install in python 3.6. Currently, the pyprismatic works for python 3.6 but it can't be installed easily.

conda-forge-linter commented 4 years ago

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

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

tschoonj commented 4 years ago

Tentative of alternative to #2. Even if I am not sure if pyprismatic needs python version specific build, in practise we can't make the recipe noarch, so we do need packages for python 3.6 and 3.7 to be able to install in python 3.6. Currently, the pyprismatic works for python 3.6 but it can't be installed easily.

pyprismatic definitely needs a python version specific build since it uses the Python internals C API extensively.

The current pyprismatic package does not work for python 3.6: check the contents of the index.json file in the tarball:

{
  "arch": "x86_64",
  "build": "cpu_hc9b8b49_0",
  "build_number": 0,
  "depends": [
    "fftw >=3.3.8,<4.0a0",
    "h5py",
    "hdf5 >=1.10.5,<1.10.6.0a0",
    "libgcc-ng >=7.3.0",
    "libstdcxx-ng >=7.3.0",
    "numpy",
    "python >=3.7,<3.8.0a0",
    "qt >=5.12.5,<5.13.0a0",
    "zlib >=1.2.11,<1.3.0a0"
  ],
  "license": "GPL-3.0-only",
  "name": "pyprismatic",
  "platform": "linux",
  "subdir": "linux-64",
  "timestamp": 1575987588035,
  "version": "1.2.1"
}

Clearly the package has a dependency on python 3.7. Even if you would have managed to install this package in a python 3.6 environment, you would have gotten a run-time linker error for sure:

$ ldd core.so 
    linux-vdso.so.1 =>  (0x00007ffd59d68000)
    libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f1fd31ce000)
    libfftw3f.so.3 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libfftw3f.so.3 (0x00007f1fd2fb6000)
    libfftw3f_threads.so.3 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libfftw3f_threads.so.3 (0x00007f1fd354b000)
    libhdf5.so.103 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libhdf5.so.103 (0x00007f1fd2c15000)
    libm.so.6 => /lib64/libm.so.6 (0x00007f1fd2913000)
    libhdf5_cpp.so.103 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libhdf5_cpp.so.103 (0x00007f1fd34d4000)
    libpython3.7m.so.1.0 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libpython3.7m.so.1.0 (0x00007f1fd25ac000)
    libstdc++.so.6 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libstdc++.so.6 (0x00007f1fd2438000)
    libgcc_s.so.1 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../../libgcc_s.so.1 (0x00007f1fd34bf000)
    libc.so.6 => /lib64/libc.so.6 (0x00007f1fd206a000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f1fd33ea000)
    librt.so.1 => /lib64/librt.so.1 (0x00007f1fd1e62000)
    libz.so.1 => /dls_sw/apps/python/anaconda/4.6.14/64/envs/epsic3.7/lib/python3.7/site-packages/pyprismatic/./../../.././libz.so.1 (0x00007f1fd34a3000)
    libdl.so.2 => /lib64/libdl.so.2 (0x00007f1fd1c5e000)
    libutil.so.1 => /lib64/libutil.so.1 (0x00007f1fd1a5b000)

This clearly shows that the core.so module is linked against the python 3.7 shared library regardless of the environment it ends up in, which probably should not have happened in the first place, see here and links therein.

ericpre commented 4 years ago

@tschoonj: the main difference between this PR and https://github.com/conda-forge/prismatic_split-feedstock/pull/2 is that the matrix build for python 3.6 is added on azure pipeline: see change in commit https://github.com/conda-forge/prismatic_split-feedstock/pull/3/commits/d594b1ab2cd57220d29bd631005b200f11092745 and the following re-rendering add the python 3.6 build https://github.com/conda-forge/prismatic_split-feedstock/pull/3/commits/af5b376d23043beca0bb3828813e07776475616b.

If you rerender https://github.com/conda-forge/prismatic_split-feedstock/pull/2, there will be no changes to the feedstock CI configuration and no python 3.6 build will be added; https://github.com/conda-forge/prismatic_split-feedstock/pull/3/commits/d594b1ab2cd57220d29bd631005b200f11092745 is one way to do it.

I have tried the brute force approach of installing the current pyprismatic and running it from python 3.6 and it works. Regardless of this, I agree this is safer and more straightforward to make two different builds considering that the build time is fairly short.

tschoonj commented 4 years ago

@tschoonj: the main difference between this PR and #2 is that the matrix build for python 3.6 is added on azure pipeline: see change in commit d594b1a and the following re-rendering add the python 3.6 build af5b376.

My PR also adds the 3.6 build. If you have a look at the Azure build log, you can see at the very end that pyprismatic packages are created for both versions of python.

If you rerender #2, there will be no changes to the feedstock CI configuration and no python 3.6 build will be added; d594b1a is one way to do it.

Indeed rerendering doesn't make any changes in my PR, and yet I still get pyprismatic builds for both 3.6 and 3.7.

I have tried the brute force approach of installing the current pyprismatic and running it from python 3.6 and it works. Regardless of this, I agree this is safer and more straightforward to make two different builds considering that the build time is fairly short.

There is no way that can work for the reasons I explained in my last post. You must be using python 3.7 somehow.

tschoonj commented 4 years ago

You have also split up the macOS builds into python specific jobs, which is unnecessary since the pyprismatic is currently disabled for that platform.

douglowe commented 4 years ago

hi guys - sorry I've not looked at this yet, been distracted with teaching yesterday. I will have a look at the suggested changes today & tomorrow, and make more detailed comments once I've got my head around them. I do agree, though, that it's wise to make clear the python version in the pyprismatic build.

ericpre commented 4 years ago

My PR also adds the 3.6 build. If you have a look at the Azure build log, you can see at the very end that pyprismatic packages are created for both versions of python.

Indeed I missed that, even if I thought I checked what package was build :/. Agree that #2 is a significantly better approach then!