sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.41k stars 475 forks source link

Doctest failure when SAGE_LIB doesn't match `.../site-packages` #33532

Closed tornaria closed 2 years ago

tornaria commented 2 years ago

After #33473, the doctest for the function sage.env.sage_include_directories() will fail when SAGE_LIB doesn't include site-packages.

This happens in editable installs (./configure --enable-editable):

$ ./sage -t src/sage/env.py
sage -t --random-seed=48380190928639724450052674778554752620 src/sage/env.py
**********************************************************************
File "src/sage/env.py", line 359, in sage.env.sage_include_directories
Failed example:
    sage.env.sage_include_directories()
Expected:
    ['.../site-packages',
     '.../site-packages/numpy/core/include',
     '.../include/python...']
Got:
    ['/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src',
     '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/numpy/core/include',
     '/usr/local/opt/python@3.10/Frameworks/Python.framework/Versions/3.10/include/python3.10']

Likewise in the nonstandard configuration of running sagelib without installing, from pkgs/sagemath-standard/build:

$ export PATH=$PWD/pkgs/sagemath-standard/build/scripts-3.10:$PATH
$ export PYTHONPATH=$PWD/pkgs/sagemath-standard/build/lib.linux-x86_64-3.10
$ sage -t src/sage/env.py 
...
**********************************************************************
File "src/sage/env.py", line 359, in sage.env.sage_include_directories
Failed example:
    sage.env.sage_include_directories()
Expected:
    ['.../site-packages',
     '.../site-packages/numpy/core/include',
     '.../include/python...']
Got:
    ['/opt/sage/sage-git/develop/pkgs/sagemath-standard/build/lib.linux-x86_64-3.10',
     '/usr/lib/python3.10/site-packages/numpy/core/include',
     '/usr/include/python3.10']
**********************************************************************
...

Component: doctest framework

Author: Gonzalo Tornaría

Branch/Commit: 0463009

Reviewer: Matthias Koeppe

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

tornaria commented 2 years ago

Commit: 0463009

tornaria commented 2 years ago

Author: Gonzalo Tornaría

tornaria commented 2 years ago

Branch: u/tornaria/33532

tornaria commented 2 years ago
comment:1

I changed the doctest for

        sage: import sage.env
        sage: sage.env.sage_include_directories()
        ['...',
         '.../numpy/core/include',
         '.../include/python...']

removing site-packages from both the first and second entry. I'm not sure python always uses site-packages so it's better to be safe. What's being tested here is that the numpy include location comes before the python include location. The first entry is the sage lib directory (either SAGE_SRC or SAGE_LIB) so we don't really know anything about it.


New commits:

0463009fix doctest for sage.env.sage_include_directories()
mkoeppe commented 2 years ago
comment:2

I'm not sure if this should be a supported configuration

fchapoton commented 2 years ago
comment:3

this doctest failure appears in our "build and test" box above here and in every ticket

tornaria commented 2 years ago
comment:4

Replying to @mkoeppe:

I'm not sure if this should be a supported configuration

Why not? It works like a charm. I build sagelib from a git repo, and run from there. No longer do I waste my time with spkgs (everything needed is installed in my system).

This is also how I build sagemath in void linux so that doctest runs at build time (and also CI on github whenever I push an update).

Maybe I should learn how to run sagelib "in place" so that I don't need to rebuild when editing *.py files -- that would be even better. Still the path won't have site-packages so the current issue will still happen. For the distro build, using a clean directory for build (and testing the build) seems more robust.

Here's my shell script, in case someone else finds it useful, I call it sagelib and place it in the root of the git checkout:

#! /bin/sh

set -e

sage_build() {
    # bootstrap sagelib
    ( cd build/pkgs/sagelib && \
        PATH=../../bin:$PATH \
        ./bootstrap )

    ( cd pkgs/sagemath-standard &&
        PYTHONPATH=../sage-setup \
        SAGE_NUM_THREADS=0 \
        python setup.py build )

    SAGE_SCRIPTS=$(cd pkgs/sagemath-standard/build/scripts* && pwd)
    SAGE_LIB=$(cd pkgs/sagemath-standard/build/lib* && pwd)

    # remove sage-venv-config since it clobbers PATH
    rm $SAGE_SCRIPTS/sage-venv-config

    # override pytest as if not installed
    echo "raise ModuleNotFoundError" > $SAGE_LIB/pytest.py

    # create sage_conf.py
    sed '1,/^exit/d' "$0" > $SAGE_LIB/sage_conf.py
}

if [ "$1" = "build" ]; then
    sage_build
    exit
fi

export PATH=$(cd pkgs/sagemath-standard/build/scripts* && pwd):$PATH
export PYTHONPATH=$(cd pkgs/sagemath-standard/build/lib* && pwd)

sage "$@"

exit $?
# configuration for sage on void linux
CONWAY_POLYNOMIALS_DATA_DIR = "/usr/share/sagemath/conway_polynomials"
GRAPHS_DATA_DIR = "/usr/share/sagemath/graphs"
ELLCURVE_DATA_DIR = "/usr/share/sagemath/ellcurves"
POLYTOPE_DATA_DIR = "/usr/share/sagemath/reflexive_polytopes"
COMBINATORIAL_DESIGN_DATA_DIR = "/usr/share/sagemath/combinatorial_designs"
CREMONA_MINI_DATA_DIR = "/usr/share/sagemath/cremona"
CREMONA_LARGE_DATA_DIR = "/usr/share/sagemath/cremona"
GAP_SO = "libgap.so.0"

Just run ./sagelib build to build, and ./sagelib to run sage. I can successfully run ./sagelib -t --long --all with this on a clean checkout of 9.6.beta5 except for the current issue and a few more minor doctest failures (mostly deprecation warnings due to newer python packages).

With a primed ccache and 36 cores building this from scratch takes less time than running ./configure once.

mkoeppe commented 2 years ago
comment:5

Your static configuration of these variables ("configuration for sage on void linux ") is exactly what I had in mind when I recommended that downstream packagers provide their own version of sage_conf.py, which would make these settings persistent instead of relying on a script that sets environment variables.

See also #33295 (Refactor sage_conf)

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,25 @@
 After #33473, the doctest for the function `sage.env.sage_include_directories()` will fail when `SAGE_LIB` doesn't include `site-packages`.

-For example, running sagelib without installing, from `pkgs/sagemath-standard/build`:
+This happens in editable installs (`./configure --enable-editable`):
+
+```
+
+sage -t --random-seed=48380190928639724450052674778554752620 src/sage/env.py
+**********************************************************************
+File "src/sage/env.py", line 359, in sage.env.sage_include_directories
+Failed example:
+    sage.env.sage_include_directories()
+Expected:
+    ['.../site-packages',
+     '.../site-packages/numpy/core/include',
+     '.../include/python...']
+Got:
+    ['/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/src',
+     '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/numpy/core/include',
+     '/usr/local/opt/python@3.10/Frameworks/Python.framework/Versions/3.10/include/python3.10']
+```
+$ ./sage -t src/sage/env.py
+Likewise in the nonstandard configuration of running sagelib without installing, from `pkgs/sagemath-standard/build`:

$ export PATH=$PWD/pkgs/sagemath-standard/build/scripts-3.10:$PATH

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -3,7 +3,7 @@
 This happens in editable installs (`./configure --enable-editable`):

- +$ ./sage -t src/sage/env.py sage -t --random-seed=48380190928639724450052674778554752620 src/sage/env.py


File "src/sage/env.py", line 359, in sage.env.sage_include_directories @@ -18,7 +18,7 @@ '/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/numpy/core/include', '/usr/local/opt/python@3.10/Frameworks/Python.framework/Versions/3.10/include/python3.10']

-$ ./sage -t src/sage/env.py
+
 Likewise in the nonstandard configuration of running sagelib without installing, from `pkgs/sagemath-standard/build`:
mkoeppe commented 2 years ago
comment:8

This failure also happens in editable installs. Thanks for catching this (we don't test this automatically yet - #31413)

mkoeppe commented 2 years ago

Reviewer: Matthias Koeppe

tornaria commented 2 years ago
comment:10

Replying to @mkoeppe:

Your static configuration of these variables ("configuration for sage on void linux ") is exactly what I had in mind when I recommended that downstream packagers provide their own version of sage_conf.py, which would make these settings persistent instead of relying on a script that sets environment variables.

Before today I was appending all of that to sage/env.py, and you already recommended to me that I should provide a sage_conf package, and that's also what pkgs/sage-conf/README.rst kind of suggest. But I completely misunderstood you: I thought it was a chore to have to build a separate package sage_conf using what's in pkgs/sage-conf. It wasn't until today that I realized it was a simple matter of writing all that stuff to sage_conf.py!!! If that's what you meant, it may be a good idea to give it as a suggestion/example.

Thanks for the quick review.

mkoeppe commented 2 years ago
comment:11

Yes, and with #33295 it will become even simpler

vbraun commented 2 years ago

Changed branch from u/tornaria/33532 to 0463009