Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New doctest option to hide features #34185

Closed
soehms opened this issue Jul 15, 2022 · 28 comments · Fixed by #35668
Closed

New doctest option to hide features #34185

soehms opened this issue Jul 15, 2022 · 28 comments · Fixed by #35668

Comments

@soehms
Copy link
Member

soehms commented Jul 15, 2022

As suggested in #33823 (see comment 25) I make a first attempt for such an option here. The purpose of the option is to find test which need to be marked as optional but aren't right now.

For example see the test of the following file which contains two tests involving the optional package database_knotinfo. Note that on the system where I produced the output below this feature is available.

cat test_knotinfo.py
r"""
sage: KnotInfo.K9_10.homfly_polynomial()
-v^10*z^2 + v^8*z^4 - 2*v^10 + 2*v^8*z^2 + 2*v^6*z^4 + v^8 + 5*v^6*z^2 + v^4*z^4 + 2*v^6 + 2*v^4*z^2
sage: KnotInfo.K9_10.alexander_polynomial()  # optional database_knotinfo
4*t^4 - 8*t^3 + 9*t^2 - 8*t + 4
"""

The first test is missing the optional tag. Thus, this would fail on a system without that feature. But, running sage -t without the new option does not show the problem:

sage -t test_knotinfo.py
too many failed tests, not using stored timings
Running doctests with ID ...
...
Git ref: 9.7.beta5-...
...
Using --optional=database_knotinfo,debian,debugpy,mathics,mathics_scanner,palettable,pint,pip,sage,sage_spkg
Features to be detected: ...
Doctesting 1 file.
sage -t --random-seed=17350038375103993251556014960108337039 test_knotinfo.py
    [2 tests, 4.66 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 4.7 seconds
    cpu time: 4.7 seconds
    cumulative wall time: 4.7 seconds
Features detected for doctesting:

Using the new option this will be the case:

sage -t --hide=database_knotinfo test_knotinfo.py
too many failed tests, not using stored timings
Running doctests with ...
...
Git ref: 9.7.beta5-...
...
Using --optional=debian,debugpy,mathics,mathics_scanner,palettable,pint,pip,sage,sage_spkg
Features to be detected: ...
Hidden features: database_knotinfo
Doctesting 1 file.
sage -t --random-seed=109756173219998976577146490280531589198 test_knotinfo.py
**********************************************************************
File "test_knotinfo.py", line 2, in test_knotinfo
Failed example:
    KnotInfo.K9_10.homfly_polynomial()
Exception raised:
    Traceback (most recent call last):
    ...
    NameError: name 'KnotInfo' is not defined
**********************************************************************
1 item had failures:
   1 of   2 in test_knotinfo
    [1 test, 1 failure, 0.00 s]
----------------------------------------------------------------------
sage -t --random-seed=109756173219998976577146490280531589198 test_knotinfo.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.0 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.0 seconds
Features detected for doctesting:

Depends on #34282
Depends on #34283

CC: @seblabbe

Component: doctest framework

Author: Sebastian Oehms

Branch/Commit: u/soehms/hide_features_34185 @ 2a582f4

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

@soehms soehms added this to the sage-9.7 milestone Jul 15, 2022
@soehms
Copy link
Member Author

soehms commented Jul 15, 2022

Branch: u/soehms/hide_features_34185

@soehms
Copy link
Member Author

soehms commented Jul 15, 2022

Author: Sebastian Oehms

@soehms
Copy link
Member Author

soehms commented Jul 15, 2022

Commit: f51f29a

@soehms
Copy link
Member Author

soehms commented Jul 15, 2022

New commits:

f51f29a34185: initial version

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 15, 2022

comment:3

I think instead of reading from SAGE_PKGS directly, it would be better to refactor sage.misc.package.list_packages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2022

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

3071dbc34185: correction according to review

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2022

Changed commit from f51f29a to 3071dbc

@soehms
Copy link
Member Author

soehms commented Jul 18, 2022

comment:5

Replying to @mkoeppe:

I think instead of reading from SAGE_PKGS directly, it would be better to refactor sage.misc.package.list_packages

Actually, I didn't do that well. Many thanks for looking at the ticket!

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2022

comment:6

A JoinFeature should probably hide/unhide its components

@soehms
Copy link
Member Author

soehms commented Jul 20, 2022

comment:7

Replying to @mkoeppe:

A JoinFeature should probably hide/unhide its components

Will be in the next commit.

I'm wondering what's wrong with the patchbots? For example:

**********************************************************************
File "/Users/jpalmier/Desktop/Sage/git/patchbot/sage-9.7.beta5/src/sage/features/__init__.py", line 301, in sage.features.Feature.is_optional
Failed example:
    DatabaseCremona().is_optional()
Exception raised:
    Traceback (most recent call last):
    ...
    AttributeError: 'DatabaseCremona' object has no attribute 'is_optional'

All tests failing on the patchbots succeed in the Gitpod environment. But here there is a failing test, as well, but obviously unrelated to the branch. It is caused by:

sage: from sage.features.latex import LaTeXPackage
sage: LaTeXPackage("tkz-graph").is_present()
Traceback (most recent call last):
...
FileNotFoundError: [Errno 2] No such file or directory: 'kpsewhich'

This happens since kpsewhich is not installed on the environment. Shouldn't LaTeXPackage be a joined feature with pdflatex?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 25, 2022

Changed commit from 3071dbc to b4b562c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 25, 2022

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

faacd11Merge branch 'u/soehms/hide_features_34185' of trac.sagemath.org:sage into hide_features_34185
b4b562c34185: take care of joined features

@soehms
Copy link
Member Author

soehms commented Jul 25, 2022

comment:10

The recent commit takes care of joined features. In addition (in order to allow more interesting doctest examples) I make lazy_import be hide-able.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 5, 2022

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

70abb3fMerge branch 'u/soehms/hide_features_34185' of trac.sagemath.org:sage into hide_features_34185
c599e4934282: initial
442fca6Merge branch 'join_feature_texfile_with_pdflatex_34282' into hide_features_34185
d4cf50434283: initial
f39836aMerge branch 'circular_import_matrix_space_34283' into hide_features_34185

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 5, 2022

Changed commit from b4b562c to f39836a

@soehms
Copy link
Member Author

soehms commented Aug 5, 2022

comment:12

Replying to @soehms:

... But here there is a failing test, as well, but obviously unrelated to the branch. It is caused by:

sage: from sage.features.latex import LaTeXPackage
sage: LaTeXPackage("tkz-graph").is_present()
Traceback (most recent call last):
...
FileNotFoundError: [Errno 2] No such file or directory: 'kpsewhich'

This happens since kpsewhich is not installed on the environment. Shouldn't LaTeXPackage be a joined feature with pdflatex?

See #34282 for a suggestion to have this fixed.

Furthermore, when testing on a system with meataxe you may randomly run into a circular import issue in matrix_space. This is fixed in ticket #34283.

@soehms
Copy link
Member Author

soehms commented Aug 5, 2022

Dependencies: #34282 #34283

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2022

Changed commit from f39836a to 6912c1c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2022

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

ddd7adaMerge branch 'u/soehms/hide_features_34185' of trac.sagemath.org:sage into hide_features_34185
0ec6d46Merge branch 'u/soehms/hide_features_34185' of trac.sagemath.org:sage into hide_features_34185
7358dc234282: dd spkg='texlive', refactoring, pycodestyle
f463ac7Merge branch 'join_feature_texfile_with_pdflatex_34282' into hide_features_34185
53d8d1134283: fix according to review
ebbd4a4Merge branch 'circular_import_matrix_space_34283' into hide_features_34185
6912c1c34185: fix doctest failure + pep8 fixes

@soehms
Copy link
Member Author

soehms commented Aug 9, 2022

comment:14

The previous commit should fix the only failure that happened on the previous patchbot run. This is the following long test:

Doctesting 1 file using 4 threads.
sage -t --long --random-seed=32647687899858611983071143867735298580 src/sage/doctest/sources.py
**********************************************************************
File "src/sage/doctest/sources.py", line 781, in sage.doctest.sources.FileDocTestSource._test_enough_doctests
Failed example:
    for path, dirs, files in itertools.chain(os.walk('sage'), os.walk('doc')): # long time
        path = os.path.relpath(path)
        dirs.sort(); files.sort()
        for F in files:
            _, ext = os.path.splitext(F)
            if ext in ('.py', '.pyx', '.pxd', '.pxi', '.sage', '.spyx', '.rst'):
                filename = os.path.join(path, F)
                FDS = FileDocTestSource(filename, DocTestDefaults(long=True, optional=True, force_lib=True))
                FDS._test_enough_doctests(verbose=False)
Expected:
    There are 3 unexpected tests being run in sage/doctest/parsing.py
    There are 1 unexpected tests being run in sage/doctest/reporting.py
Got:
    There are 7 tests in sage/doctest/control.py that are not being run
    There are 3 unexpected tests being run in sage/doctest/parsing.py
    There are 1 unexpected tests being run in sage/doctest/reporting.py
**********************************************************************
1 item had failures:
   1 of   9 in sage.doctest.sources.FileDocTestSource._test_enough_doctests
    [371 tests, 1 failure, 105.47 s]
----------------------------------------------------------------------
sage -t --long --random-seed=32647687899858611983071143867735298580 src/sage/doctest/sources.py  # 1 doctest failed

@seblabbe
Copy link
Contributor

comment:15

I saw one typo when looking at the diff:

    def unhide(self):
        r"""
        Hide this feature and all its joined features.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 6, 2022

Changed commit from 6912c1c to 187bdcf

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 6, 2022

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

2e11970Merge branch 'u/soehms/hide_features_34185' of trac.sagemath.org:sage into hide_features_34185
a5900acMerge branch 'develop' into t/34282/join_feature_texfile_with_pdflatex_34282
9e6ca2aReplace pdflatex to latex
553f6ffDo not use JoinFeature
a31205aA tiny edit
dd62d0bMerge branch 'u/klee/join_feature_texfile_with_pdflatex_34282' of trac.sagemath.org:sage into join_feature_texfile_with_pdflatex_34282
a77ff0c34282: pyflakes and missing import in doctest
d676f79Merge branch 'u/soehms/join_feature_texfile_with_pdflatex_34282' of trac.sagemath.org:sage into hide_features_34185
187bdcf34185: fix typos

@soehms
Copy link
Member Author

soehms commented Sep 6, 2022

comment:17

Replying to Sébastien Labbé:

I saw one typo when looking at the diff:

    def unhide(self):
        r"""
        Hide this feature and all its joined features.

Done!

In Addition I rebased over 9.7.rc0 and the current changes in #34282.

@mkoeppe mkoeppe removed this from the sage-9.7 milestone Sep 19, 2022
@mkoeppe mkoeppe added this to the sage-9.8 milestone Sep 19, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 30, 2022

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

2a582f4Merge branch 'u/soehms/hide_features_34185' of trac.sagemath.org:sage into hide_features_34185

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 30, 2022

Changed commit from 187bdcf to 2a582f4

@soehms
Copy link
Member Author

soehms commented Sep 30, 2022

comment:20

Just rebasing to 9.8.beta1.

@soehms
Copy link
Member Author

soehms commented May 23, 2023

This former Trac ticket will be continued in PR #35668.

@vbraun vbraun closed this as completed in a6ab2f6 Jul 1, 2023
@mkoeppe mkoeppe added this to the sage-10.1 milestone Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants