Add snappy as a pip package

[mkoeppe]

Following the instructions from #31176.

(see also https://snappy.math.uic.edu/installing.html#sagemath)

Depends on #31132

Upstream: Reported upstream. No feedback yet.

CC: @NathanDunfield @culler @dimpase @videlec @slel

Component: packages: optional

Author: Matthias Koeppe

Branch/Commit: fb5366d

Reviewer: Nathan Dunfield

Issue created by migration from https://trac.sagemath.org/ticket/31180

[mkoeppe]

Branch: u/mkoeppe/add_snappy_as_a_pip_package

[sagetrac-git]

Commit: 46f7b0e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

46f7b0e

build/pkgs/snappy: New pip package

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

New commits:

46f7b0e

build/pkgs/snappy: New pip package

[mkoeppe]

comment:4

Unfortunately --no-deps is not valid syntax in requirements.txt files

[NathanDunfield]

comment:5

The full list of outside dependencies for snappy are:

pypng
decorator
future
ipython
cypari|cypari2

so just using snappy spherogram plink FXrays pypng should do the trick (note I forgot about pypng in my post on #31176). But maybe include decorator and future just in case those aren't part of some future version of Sage.

[NathanDunfield]

comment:7

Hmm, I see what you mean by --no-deps. I guess one can only give that flag to pip but not embed it in requirements.txt.

[mkoeppe]

comment:8

Looking at snappy's setup.py, I see that you are already trying to conditionalize away the issue with cypari.

 install_requires = ['plink>=2.3.1', 'spherogram>=1.8.3', 'FXrays>=1.3',
                    'pypng', 'decorator', 'future', 'snappy_manifolds>=1.1.1']
try:
    import sage
except ImportError:
    install_requires.append('cypari>=2.2')
    install_requires.append('ipython>=0.13')
    if sys.version_info < (3, 4):
        install_requires.append('ipython<6.0')
    if sys.platform == 'win32':
        install_requires.append('pyreadline>=2.0')

Unfortunately I think the presence of wheels for snappy on PyPI will circumvent it.

[mkoeppe]

comment:9

Would making the cypari dependency an extras_require in snappy be an acceptable way forward for you?

[NathanDunfield]

comment:10

Yeah, with binary wheels the install_requires are baked in and our tests in setup.py are of no help.

I don't think extras_require helps here as cypari* is a core dependency for snappy and so is in no sense optional.

[NathanDunfield]

comment:11

It's inelegant, but I suspect that, in light of pip's limitations, the best thing is simply to make requirements.txt be the single line snappy. If wheels are available, this will result in a completely unused cypari package, but this will not interfere with anything and just wastes 30M (on linux, unpacked), which is small in context snappy (165 M), much less Sage itself (2G+). (If instead pip ends up building from source, then there is no issue at all because of our setup.py.)

This also has the advantage it makes it less likely that this Sage pip package will need updating.

This argument is especially compelling if we throw in the optional database snappy_15_knots into this Sage pip package, as that is another 110M uncompressed.

[mkoeppe]

comment:12

I am not sure. It does not seem like a very robust solution.

Just as a data point: I seem to be having trouble installing cypari into Sage here on macOS using python 3.9, for which no prebuilt wheel is available.

Building wheels for collected packages: cypari
  Created temporary directory: /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-wheel-8xuqmqye
  Building wheel for cypari (setup.py) ...   Destination directory: /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-wheel-8xuqmqye
  Running command /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/python3 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/setup.py'"'"'; __file__='"'"'/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' bdist_wheel -d /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-wheel-8xuqmqye
  running bdist_wheel
  running build
  running build_py
  creating build
  creating build/lib.macosx-10.15-x86_64-3.9
  creating build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/version.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/memory.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/__init__.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/test.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/tests.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/py3tests.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  copying cypari/py2tests.py -> build/lib.macosx-10.15-x86_64-3.9/cypari
  running build_ext
  error: [Errno 2] No such file or directory: 'cypari/_pari_py3.c' -> 'cypari/_pari.c'
error
  ERROR: Failed building wheel for cypari
...
Installing collected packages: cypari
  Created temporary directory: /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-record-5v39xxkr
    Running command /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/python3 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/setup.py'"'"'; __file__='"'"'/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-record-5v39xxkr/install-record.txt --single-version-externally-managed --compile --install-headers /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/site/python3.9/cypari
    running install
    running build
    running build_py
    running build_ext
    Building gmp ...
    rm -f gp-dyn
    /Library/Developer/CommandLineTools/usr/bin/ld  -o gp-dyn -L"/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/build/pari_src/Odarwin-x86_64" -search_paths_first -L/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib -Wl,-rpath,/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib  emacs.o gp.o gp_rl.o texmacs.o whatnow.o plotX.o  -lreadline -lpari -L/usr/X11R6/lib -lX11
    ld: unknown option: -Wl,-rpath,/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib
    make: *** [gp-dyn] Error 1
    ***Failed to build PARI library***
    Running setup.py install for cypari ... error
ERROR: Command errored out with exit status 1: /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/python3 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/setup.py'"'"'; __file__='"'"'/private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-install-sbgfgcz3/cypari/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-record-5v39xxkr/install-record.txt --single-version-externally-managed --compile --install-headers /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/site/python3.9/cypari Check the logs for full command output.
Exception information:
Traceback (most recent call last):
  File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib/python3.9/site-packages/pip/_internal/req/req_install.py", line 838, in install
    success = install_legacy(
  File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib/python3.9/site-packages/pip/_internal/operations/install/legacy.py", line 86, in install
    raise LegacyInstallFailure
pip._internal.operations.install.legacy.LegacyInstallFailure

[mkoeppe]

comment:13

Also ./sage -pip install -v -v snappy on the same system gives:

  ...
  gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I/usr/local/include -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers -Ikernel/headers -Ikernel/unix_kit -Ikernel/addl_code -Ikernel/real_type -I/usr/local/include -I/usr/local/opt/openssl@1.1/include -I/usr/local/opt/sqlite/include -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include -I/usr/local/Cellar/python@3.9/3.9.1_1/Frameworks/Python.framework/Versions/3.9/include/python3.9 -c kernel/kernel_code/Dirichlet.c -o build/temp.macosx-10.15-x86_64-3.9/kernel/kernel_code/Dirichlet.o
  kernel/kernel_code/Dirichlet.c:83:10: warning: non-portable path to file '"dirichlet.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  #include "Dirichlet.h"
           ^~~~~~~~~~~~~
           "dirichlet.h"
  kernel/kernel_code/Dirichlet.c:118:91: error: unknown type name 'MatrixPairList'; did you mean 'MatrixParity'?
  static void         array_to_matrix_pair_list(O31Matrix generators[], int num_generators, MatrixPairList *gen_list);
                                                                                            ^~~~~~~~~~~~~~
                                                                                            MatrixParity
  kernel/headers/SnapPea.h:100:3: note: 'MatrixParity' declared here
  } MatrixParity;
    ^
  kernel/kernel_code/Dirichlet.c:119:52: error: unknown type name 'MatrixPairList'; did you mean 'MatrixParity'?
  static Boolean      is_matrix_on_list(O31Matrix m, MatrixPairList *gen_list);
                                                     ^~~~~~~~~~~~~~
                                                     MatrixParity
  kernel/headers/SnapPea.h:100:3: note: 'MatrixParity' declared here
  } MatrixParity;
    ^
  kernel/kernel_code/Dirichlet.c:120:56: error: unknown type name 'MatrixPairList'; did you mean 'MatrixParity'?
  static void         insert_matrix_on_list(O31Matrix m, MatrixPairList *gen_list);
                                                         ^~~~~~~~~~~~~~
                                                         MatrixParity

[NathanDunfield]

comment:14

Replying to @mkoeppe:

I am not sure. It does not seem like a very robust solution.

Not that it means much, but we haven't had any reports of problems with sage -pip install snappy as our default install instructions.

I agree it would be preferable just to pass the --no-deps flag to pip, but my understanding is that this is not possible in the context of a "sage pip package"?

Just as a data point: I seem to be having trouble installing cypari into Sage here on macOS using python 3.9, for which no prebuilt wheel is available.

Ouch, that's a bug on our part, it seems we broke the sdist tarball in 2.4.0, on all platforms, no less: we moved some files around and forgot to update MANIFEST.in. We'll release 2.4.1 to fix this shortly.

[NathanDunfield]

comment:15

Replying to @mkoeppe:

Also ./sage -pip install -v -v snappy on the same system gives:

Marc Culler points out the likely cause of this: there is a dirichlet.h in $SAGELOCAL/include that is part of arb as well as a Dirichlet.h that's part of SnapPy. Since macOS file system default to case-insensitive, there's a name collision which triggers the nonportable-include-path warning.

[mkoeppe]

comment:16

Replying to @NathanDunfield:

Replying to @mkoeppe:

Also ./sage -pip install -v -v snappy on the same system gives:

Marc Culler points out the likely cause of this: there is a dirichlet.h in $SAGELOCAL/include that is part of arb as well as a Dirichlet.h that's part of SnapPy. Since macOS file system default to case-insensitive, there's a name collision which triggers the nonportable-include-path warning.

I think it's actually coming from /usr/local/include/ on my system (from homebrew's arb).

The question is where the first -I/usr/local/include on the gcc command line is coming from. This is causing the wrong file to be picked up. (The rest of the order of the includes looks fine.)

[sagetrac-git]

Changed commit from 46f7b0e to 5a76fdc

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

5a76fdc

build/pkgs/snappy: Update dependencies, requirements.txt, add comments

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

f2ca439

No future

[sagetrac-git]

Changed commit from 5a76fdc to f2ca439

[sagetrac-git]

Changed commit from f2ca439 to c8583bb

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c8583bb

build/pkgs/snappy/requirements.txt: Avoid cypari 2.4.0

[NathanDunfield]

comment:20

Replying to @mkoeppe:

The question is where the first -I/usr/local/include on the gcc command line is coming from. This is causing the wrong file to be picked up. (The rest of the order of the includes looks fine.)

On my macOS 10.15 box, with Python in /Frameworks from Python.org, I get:

gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -arch x86_64 -g 
   -Ikernel/headers -Ikernel/unix_kit -Ikernel/addl_code -Ikernel/real_type 
   -I/Library/Frameworks/Python.framework/Versions/3.9/include/python3.9 -c 
   kernel/addl_code/get_gluing_equations.c -o build/temp.macosx-10.9-x86_64-3.9/kernel/addl_code/get_gluing_equations.o

In particular, the includes start with the SnapPy specific ones and then the location of Python.h as desired. Maybe your first -I /usr/local/include is somehow coming from CFLAGS, or from the CFLAGS that were set when Python itself was built? I believe setuptools uses as least some of the latter.

[NathanDunfield]

comment:21

Replying to @NathanDunfield:

Maybe your first -I /usr/local/include is somehow coming from CFLAGS, or from the CFLAGS that were set when Python itself was built? I believe setuptools uses as least some of the latter.

Assuming you're using homebrew's Python, the above seems quite likely as there I get:

$ /usr/local/Cellar/python@3.9/3.9.0_1/bin/python3
>>> import sysconfig
>>> sysconfig.get_config_var('CFLAGS')
'-Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall 
-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include 
-I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers'

[mkoeppe]

comment:22

Yes, my python3 is (Sage's venv over) homebrew python3.9 -- so this (the CFLAGS from sysconfig, not the environment) is how the -I/usr/local/include is entering on my system.

This is rather unfortunate and may also be the cause of the issues reported in #31132.

[dimpase]

comment:23

I have opened 3-manifolds/SnapPy#21 and
commented on flintlib/arb#239 - where the issue of arb header names was already brought up as causing problems.

[dimpase]

Upstream: Reported upstream. No feedback yet.

[mkoeppe]

Dependencies: #31132

[sagetrac-git]

Changed commit from c8583bb to fb5366d

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

2c60025	m4/sage_check_python_for_venv.m4: Warn if python3 is misconfigured like it is currently in homebrew
ee679bd	build/pkgs/python3/spkg-configure.m4: Do not check sysconfig CPPFLAGS so that /usr/bin/python3 is accepted on macOS; reject broken homebrew python3 unless requested explicitly with configgure --with-python=...
fb5366d	Merge branch 't/31132/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_build_when_using_homebrew_s_python3' into t/31180/add_snappy_as_a_pip_package

[mkoeppe]

comment:27

With the fix from #31132 (rejecting broken homebrew python3) and rejecting the broken cypari 2.4.0 via requirements.txt, this now seems to work well here on macOS.

[mkoeppe]

Reviewer: https://github.com/mkoeppe/sage/actions/runs/473272399, https://github.com/mkoeppe/sage/actions/runs/473272391, ...

[mkoeppe]

comment:30

Waiting for review

[NathanDunfield]

comment:31

Tried it out, sage -i snappy installs working snappy as expected, setting positive review.

[videlec]

comment:32

I am very glad that we will have better support for snappy in Sage!

[mkoeppe]

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/473272399, https://github.com/mkoeppe/sage/actions/runs/473272391, ... to Nathan Dunfield

[slel]

comment:34

This probably makes #20739 obsolete.

[vbraun]

Changed branch from u/mkoeppe/add_snappy_as_a_pip_package to fb5366d