Feature and doctest tag for runtime cython

Question

Feature and doctest tag for runtime cython

Closed this issue 3 years ago · 36 comments

While updating the sagemath Debian package I noticed that when running the doctests for the installed sagemath package I get many failing tests such as the one below if library development files are not installed (in the example libm4ri-dev is not installed). The sagemath packages should not depend on lib*-dev packages, but sagemath is looking for pkgconfig files which are in these packages. Could this check for pkgconfig files in the test suite be avoided?

sage -t --long --random-seed=0 sage/src/sage/arith/long.pxd
**********************************************************************
File "sage/src/sage/arith/long.pxd", line 116, in sage.arith.long.integer_check_long
Failed example:
    cython('''
    from sage.arith.long cimport *
    from sage.rings.integer cimport smallInteger
    def check_long(x):
        cdef long value
        cdef int err
        cdef bint c = integer_check_long(x, &value, &err)
        if c:
            if err == 0:
                return value
            elif err == ERR_OVERFLOW:
                raise OverflowError("integer_check_long: overflow")
        elif err == ERR_TYPE:
            raise TypeError("integer_check_long: wrong type")
        elif err == ERR_INDEX:
            raise TypeError("integer_check_long: bad __index__")
        assert False
    from libc.limits cimport LONG_MIN, LONG_MAX
    def long_min():
        return smallInteger(LONG_MIN)
    def long_max():
        return smallInteger(LONG_MAX)
    ''')
Exception raised:
    Traceback (most recent call last):
      File "sage/misc/cachefunc.pyx", line 996, in sage.misc.cachefunc.CachedFunction.__call__ (build/cythonized/sage/misc/cachefunc.c:5970)
        return self.cache[k]
    KeyError: ((), ())

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/usr/lib/python3/dist-packages/sage/doctest/forker.py", line 718, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3/dist-packages/sage/doctest/forker.py", line 1137, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.arith.long.integer_check_long[0]>", line 1, in <module>
        cython('''
      File "sage/misc/lazy_import.pyx", line 362, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:4049)
        return self.get_object()(*args, **kwds)
      File "/usr/lib/python3/dist-packages/sage/misc/cython.py", line 661, in cython_compile
        return cython_import_all(tmpfile, get_globals(), **kwds)
      File "/usr/lib/python3/dist-packages/sage/misc/cython.py", line 551, in cython_import_all
        m = cython_import(filename, **kwds)
      File "/usr/lib/python3/dist-packages/sage/misc/cython.py", line 526, in cython_import
        name, build_dir = cython(filename, **kwds)
      File "/usr/lib/python3/dist-packages/sage/misc/cython.py", line 284, in cython
        standard_libs, standard_libdirs, standard_includes, aliases = _standard_libs_libdirs_incdirs_aliases()
      File "sage/misc/cachefunc.pyx", line 1001, in sage.misc.cachefunc.CachedFunction.__call__ (build/cythonized/sage/misc/cachefunc.c:6098)
        w = self.f(*args, **kwds)
      File "/usr/lib/python3/dist-packages/sage/misc/cython.py", line 50, in _standard_libs_libdirs_incdirs_aliases
        aliases = cython_aliases()
      File "/usr/lib/python3/dist-packages/sage/env.py", line 475, in cython_aliases
        aliases[var + "CFLAGS"] = pkgconfig.cflags(lib).split()
      File "/usr/lib/python3/dist-packages/pkgconfig/pkgconfig.py", line 144, in cflags
        _raise_if_not_exists(package)
      File "/usr/lib/python3/dist-packages/pkgconfig/pkgconfig.py", line 103, in _raise_if_not_exists
        raise PackageNotFoundError(package)
    pkgconfig.pkgconfig.PackageNotFoundError: m4ri not found

Depends on #33823

CC: @kiwifb @antonio-rojas @seblabbe

Component: doctest framework

Author: Matthias Koeppe

Branch/Commit: 1440273

Reviewer: Sébastien Labbé

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

Answer 1 · 2021-12-15T22:20:05.000Z

comment:1

Run-time use of compilers is a feature of some parts of the Sage library - in this case sage.misc.cython.cython. These tests necessarily depend on dev headers (and the compilers!).

We should conditionalize these doctests on a Feature.

Answer 2 · 2021-12-15T22:30:59.000Z

comment:2

That would help. Or would you say the sagemath Debian packages should depend on the dev packages to provide this feature?

Answer 3 · 2021-12-15T22:42:14.000Z

comment:3

If currently Sage is monolithic in Debian, then you should probably add these as runtime dependencies -- because the cython function is part of the standard Sage library.

Answer 4 · 2021-12-15T22:51:01.000Z

Dependencies: #32174

Answer 5 · 2021-12-15T22:54:28.000Z

comment:6

One interesting question is how used is this feature of sage, that you can compile bit of code inside it? This is clearly advanced use but if it is widespread, you definitely need to install matching dev packages.

It is not unlike R packages. Debian provides quite a lot of them by default, but if you want something not provided, you may have to install dev packages.

Answer 6 · 2021-12-15T22:58:15.000Z

Changed dependencies from #32174 to #32174, #32925

Answer 7 · 2021-12-16T05:32:53.000Z

Branch: u/mkoeppe/feature_and_doctest_tag_for_runtime_cython

Answer 8 · 2021-12-16T05:35:33.000Z

Commit: 8ecc8cc

Answer 9 · 2021-12-16T05:35:33.000Z

Author: Matthias Koeppe, ...

Answer 10 · 2021-12-16T05:35:33.000Z

comment:9

Here's a beginning. More doctests that use cython can be marked up in the same way

Last 10 new commits:

`0b26010`	`src/doc/en/reference/misc/index.rst: Add sage.features.interfaces`
`dc954a7`	`src/sage/features/__init__.py: Fix doc markup`
`fe05309`	`sage.features.pkg_systems: New, move PackageSystem and subclasses here`
`b7cd28e`	`sage.features.sagemath: Improve docstring markup`
`bd59980`	`sage.features.mip_backends: Improve doc markup`
`d364333`	`sage.features: Improve doc markup`
`547206c`	`Change "# optional: FEATURE" to "# optional - FEATURE"`
`d1d603d`	`Merge #32925`
`994df69`	`src/sage/features/cython.py: New`
`8ecc8cc`	`src/sage/misc/inherit_comparison.pyx: Mark doctests using cython # optional - sage.misc.cython`

Answer 11 · 2021-12-16T07:08:44.000Z

comment:10

How useful is this given that tests don't run at all if cython is not present?

https://github.com/sagemath/sagetrac-mirror/blob/develop/src/sage/doctest/sources.py#n27

Answer 12 · 2021-12-16T07:36:48.000Z

comment:11

This use of is_package_dir will have to be eliminated when we change packages to namespace packages in 9.6 by removing __init__.py files

Answer 13 · 2021-12-16T07:46:09.000Z

comment:12

Another import from Cython in sage.doctest.parsing is being removed in #32938.

Answer 14 · 2021-12-16T17:16:04.000Z

comment:13

Replying to @mkoeppe:

This use of is_package_dir will have to be eliminated when we change packages to namespace packages in 9.6 by removing __init__.py files

I've opened #33033 for this

Answer 15 · 2021-12-16T18:48:33.000Z

Changed dependencies from #32174, #32925 to #32174, #32925, #33033

Answer 16 · 2021-12-16T19:58:19.000Z

Changed commit from 8ecc8cc to 29962d1

Answer 17 · 2021-12-16T19:58:19.000Z

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

`c3f6bcb`	`src/sage/misc/namespace_package.py (is_package_or_sage_namespace_package_dir): New`
`ab23a14`	`src/sage/doctest/sources.py: Use is_package_or_sage_namespace_package_dir`
`0f05856`	`src/sage/doctest/sources.py: Remove unnecessary conversion to bool`
`d8aa37c`	`src/sage/misc/namespace_package.py: In doctests, use 'directory' instead of the single-letter variable name`
`29962d1`	`Merge #33033`

Answer 18 · 2021-12-17T06:16:47.000Z

Changed commit from 29962d1 to 22d8166

Answer 19 · 2021-12-17T06:16:47.000Z

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

`19e93f0`	`is_package_or_sage_namespace_package_dir: Make directories with __init__.pxd package directories`
`5808f3d`	`src/sage/misc/package_dir.py: New file for is_package_or_sage_namespace_package_dir`
`22d8166`	`Merge #33033`

Answer 20 · 2022-04-28T18:28:45.000Z

Changed commit from 22d8166 to 9986d50

Answer 21 · 2022-04-28T18:28:45.000Z

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

`c9c196c`	`src/sage/features/cython.py: New`
`9986d50`	`src/sage/misc/inherit_comparison.pyx: Mark doctests using cython # optional - sage.misc.cython`

Answer 22 · 2022-04-28T18:29:11.000Z

Changed dependencies from #32174, #32925, #33033 to none

Answer 23 · 2022-04-28T18:32:34.000Z

Work Issues: Mark more doctests # optional - sage.misc.cython

Answer 24 · 2022-05-09T03:44:17.000Z

Dependencies: #33823

Answer 25 · 2022-05-09T04:42:54.000Z

Changed commit from 9986d50 to 4764f2e

Answer 26 · 2022-05-09T04:42:54.000Z

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

`2b9dcf7`	`src/sage/features/cython.py: New`
`e439ff1`	`src/sage/misc/inherit_comparison.pyx: Mark doctests using cython # optional - sage.misc.cython`
`8000320`	`sage -t: Handle --optional=!FEATURE`
`d0a52ed`	`src/bin/sage-runtests: Update documentation of --optional`
`edb6afb`	`Merge #33823`
`4764f2e`	`Mark tests using runtime Cython # optional - sage.misc.cython`

Answer 27 · 2022-05-09T04:43:11.000Z

Changed work issues from Mark more doctests # optional - sage.misc.cython to none

Answer 28 · 2022-05-09T04:43:11.000Z

Changed author from Matthias Koeppe, ... to Matthias Koeppe

Answer 29 · 2022-05-09T04:47:43.000Z

comment:25

Test with git grep -l 'sage:.*cython(' | xargs ./sage -tp --long --optional 'sage,!sage.misc.cython'

Answer 30 · 2022-06-09T20:26:26.000Z

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

`b1ce8d6`	`sage -t: Handle --optional=!FEATURE`
`a670ae8`	`src/bin/sage-runtests: Update documentation of --optional`
`ba86146`	`src/bin/sage-runtests --help: Say that ! needs to be quoted`
`a1154c0`	`src/sage/features/cython.py: New`
`9356cb9`	`src/sage/misc/inherit_comparison.pyx: Mark doctests using cython # optional - sage.misc.cython`
`1440273`	`Mark tests using runtime Cython # optional - sage.misc.cython`

Answer 31 · 2022-06-09T20:26:26.000Z

Changed commit from 4764f2e to 1440273

Answer 32 · 2022-06-24T08:07:08.000Z

comment:28

Replying to @mkoeppe:

Test with git grep -l 'sage:.*cython(' | xargs ./sage -tp --long --optional 'sage,!sage.misc.cython'

The above command works for me:

...
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 70.5 seconds
    cpu time: 158.9 seconds
    cumulative wall time: 240.4 seconds
Features detected for doctesting: sage.groups,sage.rings.padics,sagemath_doc_html,sphinx

Answer 33 · 2022-06-24T08:13:27.000Z

comment:29

The only reported test failure is:

----------------------------------------------------------------------
sage -t --long --random-seed=338071419821968503909259763280493589917 src/sage/modular/overconvergent/hecke_series.py  # Timed out
----------------------------------------------------------------------

which is not related to this ticket.

Answer 34 · 2022-06-24T08:16:02.000Z

comment:30

Patchbot pyflakes suggests 4 corrections:

src/sage/env.py:32:1 'sage' imported but unused
src/sage/misc/cython.py:295:9 'setuptools' imported but unused
src/sage/misc/sagedoc.py:791:9 'sage.all' imported but unused
src/sage/misc/sagedoc.py:969:5 local variable 'html_results' is assigned to but never used

but I suggest to fix them in another ticket : #34061.

Answer 35 · 2022-06-24T08:16:23.000Z

Reviewer: Sébastien Labbé

Answer 36 · 2022-07-03T22:06:06.000Z

Changed branch from u/mkoeppe/feature_and_doctest_tag_for_runtime_cython to 1440273