Feature and doctest tag for runtime cython #33029

tobihan · 2021-12-15T22:10:22Z

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

The text was updated successfully, but these errors were encountered:

mkoeppe · 2021-12-15T22:20:05Z

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.

tobihan · 2021-12-15T22:30:59Z

comment:2

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

mkoeppe · 2021-12-15T22:42:14Z

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.

mkoeppe · 2021-12-15T22:51:01Z

Dependencies: #32174

kiwifb · 2021-12-15T22:54:28Z

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.

mkoeppe · 2021-12-15T22:58:15Z

Changed dependencies from #32174 to #32174, #32925

mkoeppe · 2021-12-16T05:32:53Z

Branch: u/mkoeppe/feature_and_doctest_tag_for_runtime_cython

mkoeppe · 2021-12-16T05:35:33Z

Commit: 8ecc8cc

mkoeppe · 2021-12-16T05:35:33Z

Author: Matthias Koeppe, ...

mkoeppe · 2021-12-16T05:35:33Z

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`

antonio-rojas · 2021-12-16T07:08:44Z

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

mkoeppe · 2021-12-16T07:36:48Z

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

mkoeppe · 2021-12-16T07:46:09Z

comment:12

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

mkoeppe · 2021-12-16T17:16:04Z

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

mkoeppe · 2021-12-16T18:48:33Z

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

sagetrac-git · 2021-12-16T19:58:19Z

Changed commit from 8ecc8cc to 29962d1

sagetrac-git · 2021-12-16T19:58:19Z

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`

sagetrac-git · 2021-12-17T06:16:47Z

Changed commit from 29962d1 to 22d8166

sagetrac-git · 2021-12-17T06:16:47Z

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`

sagetrac-git · 2022-04-28T18:28:45Z

Changed commit from 22d8166 to 9986d50

sagetrac-git · 2022-04-28T18:28:45Z

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`

mkoeppe · 2022-04-28T18:29:11Z

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

mkoeppe · 2022-04-28T18:32:34Z

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

mkoeppe · 2022-05-09T03:44:17Z

Dependencies: #33823

sagetrac-git · 2022-05-09T04:42:54Z

Changed commit from 9986d50 to 4764f2e

sagetrac-git · 2022-05-09T04:42:54Z

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`

mkoeppe · 2022-05-09T04:43:11Z

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

mkoeppe · 2022-05-09T04:43:11Z

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

mkoeppe · 2022-05-09T04:47:43Z

comment:25

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

sagetrac-git · 2022-06-09T20:26:26Z

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`

sagetrac-git · 2022-06-09T20:26:26Z

Changed commit from 4764f2e to 1440273

seblabbe · 2022-06-24T08:07:08Z

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

seblabbe · 2022-06-24T08:13:27Z

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.

seblabbe · 2022-06-24T08:16:02Z

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.

seblabbe · 2022-06-24T08:16:23Z

Reviewer: Sébastien Labbé

vbraun · 2022-07-03T22:06:06Z

Changed branch from u/mkoeppe/feature_and_doctest_tag_for_runtime_cython to 1440273

tobihan added this to the sage-9.5 milestone Dec 15, 2021

tobihan added c: doctest framework labels Dec 15, 2021

mkoeppe changed the title ~~tests depend on library development files~~ Feature and doctest tag for runtime cython Dec 15, 2021

mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021

mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 7, 2022

mkoeppe added the s: needs review label May 9, 2022

seblabbe added s: positive review and removed s: needs review labels Jun 24, 2022

vbraun removed the s: positive review label Jul 3, 2022

vbraun closed this as completed in b5d2fca Jul 3, 2022

seblabbe mentioned this issue Jul 9, 2022

fix pyflakes warnings in src/sage/misc/cython.py #34061

Closed

Feature and doctest tag for runtime cython #33029

Feature and doctest tag for runtime cython #33029

Comments

tobihan commented Dec 15, 2021

mkoeppe commented Dec 15, 2021

tobihan commented Dec 15, 2021

mkoeppe commented Dec 15, 2021

mkoeppe commented Dec 15, 2021

kiwifb commented Dec 15, 2021

mkoeppe commented Dec 15, 2021

mkoeppe commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

antonio-rojas commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

mkoeppe commented Dec 16, 2021

sagetrac-git mannequin commented Dec 16, 2021

sagetrac-git mannequin commented Dec 16, 2021

sagetrac-git mannequin commented Dec 17, 2021

sagetrac-git mannequin commented Dec 17, 2021

sagetrac-git mannequin commented Apr 28, 2022

sagetrac-git mannequin commented Apr 28, 2022

mkoeppe commented Apr 28, 2022

mkoeppe commented Apr 28, 2022

mkoeppe commented May 9, 2022

sagetrac-git mannequin commented May 9, 2022

sagetrac-git mannequin commented May 9, 2022

mkoeppe commented May 9, 2022

mkoeppe commented May 9, 2022

mkoeppe commented May 9, 2022

sagetrac-git mannequin commented Jun 9, 2022

sagetrac-git mannequin commented Jun 9, 2022

seblabbe commented Jun 24, 2022

seblabbe commented Jun 24, 2022

seblabbe commented Jun 24, 2022

seblabbe commented Jun 24, 2022

vbraun commented Jul 3, 2022