sage.doctest.control: Exclude doctests in files via file directives ''# sage.doctest: optional - xyz'

[mkoeppe]

When a file is marked # sage.doctest: optional - xyz, we omit it from doctesting unless --optional=xyz is given.

This will save us from having to add lots of # optional - ... tags to files in the course of modularization (#29705)

We do this by extending sage.doctest.control.skipfile, which already parses files for # nodoctest file directives.

Previous related proposals/discussions: #3260, #20427

Also related: #30746

CC: @simon-king-jena @kiwifb @roed314 @saraedum @nthiery @videlec

Component: doctest framework

Keywords: sd111

Author: Matthias Koeppe, John Palmieri

Branch/Commit: 5cc1288

Reviewer: John Palmieri, Matthias Koeppe

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

[mkoeppe]

comment:2

Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111

[mkoeppe]

Changed keywords from none to sd111

[mkoeppe]

comment:6

We could also parse a directive "# doctest: optional - ..." in the file header

[mkoeppe]

Branch: u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions

[jhpalmieri]

comment:8

Replying to @mkoeppe:

We could also parse a directive "# doctest: optional - ..." in the file header

This has been discussed in the past and rejected, on the grounds that the directive is hidden when people look at the reference manual or just do x.method?, so they may expect those tests to work all the time. For any of these proposed directives, the corresponding page in the reference manual should have a clear warning. I don't know what to do about the help string for each relevant method/function/class. It's overkill to modify the help string based on such a directive, right?

[jhpalmieri]

Changed branch from u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions to none

[jhpalmieri]

New commits:

b92e036

sage.doctest: Handle file directives '# sage.doctest: optional - xyz'

[jhpalmieri]

Branch: u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions

[jhpalmieri]

Commit: b92e036

[mkoeppe]

Changed branch from u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions to none

[mkoeppe]

Changed commit from b92e036 to none

[mkoeppe]

Branch: u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions

[mkoeppe]

Commit: b92e036

[mkoeppe]

comment:12

Replying to @jhpalmieri:

Replying to @mkoeppe:

We could also parse a directive "# doctest: optional - ..." in the file header

This has been discussed in the past and rejected

I was trying to find the ticket with this discussion but did not succeed

---

New commits:

b92e036

sage.doctest: Handle file directives '# sage.doctest: optional - xyz'

[mkoeppe]

comment:13

There is certainly an open question here. I don't think it's only a matter of marking up the doctests with "optional" tags. With modularization going forward, we need to find a way to inform the user that some Python module is provided by a particular distribution package. I think this information should be attached to the module name instead of cluttering the doctests.

[mkoeppe]

comment:14

Until we have a more systematic solution for this, I would just suggest that we add an admonition to the module documentation that says something like: "The classes in this module require the optional package rpy2 to be installed"

[mkoeppe]

comment:15

OK, found some previous discussion: #3260, #20427

[mkoeppe]

Changed dependencies from #30746 to #30746, #3260, #20427

[sagetrac-git]

Changed commit from 07818e6 to 94727df

[jhpalmieri]

comment:33

This could still use some examples, as mentioned in comment:23 and comment:26.

[mkoeppe]

Changed dependencies from #31377, #30912, #30383 to none

[mkoeppe]

comment:35

As an example, we could apply it to src/sage/matrix/matrix_gfpn_dense.pyx and remove the line-by-line annotations "# optional: meataxe".

However, as git grep 'sage:' src/sage/matrix/matrix_gfpn_dense.pyx shows, there are some doctest lines that are not marked optional. Not sure if they have any value by themselves.

[jhpalmieri]

comment:36

@simon-king-jena: would you object if we skipped all tests in src/sage/matrix/matrix_gfpn_dense.pyx unless meataxe is installed? I've skimmed through and don't see anything that (I hope) isn't tested elsewhere for other matrix implementations, but you know this file pretty well.

[simon-king-jena]

comment:37

Replying to @jhpalmieri:

@simon-king-jena: would you object if we skipped all tests in src/sage/matrix/matrix_gfpn_dense.pyx unless meataxe is installed? I've skimmed through and don't see anything that (I hope) isn't tested elsewhere for other matrix implementations, but you know this file pretty well.

I did not intend to create new tests for other matrix implementations. So, I wouldn't object to make all the module's tests optional. In fact, this is what I asked for since I first created the module.

[mkoeppe]

Changed branch from u/jhpalmieri/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions to u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions

[mkoeppe]

Changed commit from 94727df to 1276609

[mkoeppe]

New commits:

1276609

src/sage/matrix/matrix_gfpn_dense.pyx: Use # sage.doctest: optional - meataxe

[mkoeppe]

comment:40

(untested)

[jhpalmieri]

comment:41

This is working for me the way that I think it should: commands like make ptestlong or ./sage -tp src/sage/matrix/ ignore matrix_gfpn_dense.pyx, but if you specify the file explicitly (or let the shell specify it with ./sage -tp src/sage/matrix/matrix_g*), then it gets tested, and of course many tests fail. I think that's the right behavior.

[mkoeppe]

comment:42

Remains to put something in the documentation of matrix_gfpn_dense.pyx, as discussed in comment:8, comment:13, comment:14

[jhpalmieri]

comment:43

Replying to @mkoeppe:

Remains to put something in the documentation of matrix_gfpn_dense.pyx, as discussed in comment:8, comment:13, comment:14

This is easy enough to do manually. It would be nice to autodetect the line # sage.doctest: optional - meataxe, but do we want to do that here or a follow-up ticket?

[mkoeppe]

comment:44

Let's do this manually for this ticket first

[mkoeppe]

comment:45

In #32614 I now define some new optional tags, in particular a tag sage.matrix.matrix_gfpn_dense. So if we depend on that ticket, we could now write # sage.doctest: optional - sage.matrix.matrix_gfpn_dense; and in the module documentation we could refer to the class providing the feature (sage__matrix__matrix_gfpn_dense)

[sagetrac-git]

Changed commit from 1276609 to 5cc1288

[sagetrac-git]

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

5cc1288

src/sage/matrix/matrix_gfpn_dense.pyx: Add reference to meataxe spkg

[mkoeppe]

comment:47

I've kept it simple and just added a note in the docstring.

But do we actually build HTML documentation for optional compiled modules at all?

[jhpalmieri]

comment:48

I'm happy with your changes on the ticket: positive review for those parts.

[jhpalmieri]

Reviewer: John Palmieri, ...

[mkoeppe]

comment:49

Thanks!

[mkoeppe]

Changed reviewer from John Palmieri, ... to John Palmieri, Matthias Koeppe

[vbraun]

Changed branch from u/mkoeppe/sage_doctest_control__exclude_doctests_in_files_from_non_installed_distributions to 5cc1288