source code comments: doctesting instructions

[Samuel Lelièvre]

When consulting the documentation, many code fragments
have annotations that are meant as doctesting instructions.

Among these doctesting instructions, there is one, "# not tested",
that tells the doctesting mechanism to skip that line in doctests.

However, to the non-expert eye, such as a non-developer user
reading the documentation, seeing "# not tested" produces the
impression that this piece of code is not guaranteed to work,
because it has never been tested.

I would like to make a suggestion to make the comments more
explicit, by replacing them as follows:

- # not tested
+ # doctest: skip

- # not tested (random order)
+ # doctest: skip (random order)

- # optional - qepcad
+ # doctest: optional - qepcad

etc.

[Jeroen Demeyer]

On 2015-10-29 13:23, Samuel Lelièvre wrote:
> However, to the non-expert eye, such as a non-developer user
> reading the documentation, seeing "# not tested" produces the
> impression that this piece of code is not guaranteed to work,
> because it has never been tested.

That impression is 100% right.

[Eric Gourgoulhon]

Le jeudi 29 octobre 2015 13:23:56 UTC+1, Samuel Lelievre a écrit :

-     # not tested
+     # doctest: skip

-     # not tested (random order)
+     # doctest: skip (random order)

-     # optional - qepcad
+     # doctest: optional - qepcad

+1

Eric.

[Nathann Cohen]

+1. Please update the developer's manual if you change this.

Nathann

[Vincent Delecroix]

Though, randomness problem *should* be tested... we currently have a keyword

# random

that does execute the code but does not try to match the result with
what is written in the doctest. Following your suggestion this would become

- # random
+ # doctest: random

Vincent

[Nathann Cohen]

Arggggg... Sorry I misread. I have to live with half a brain these days.

I'm +1 to the change 'not tested' -> 'skip' for the reason that you
gave in your first message.

I'm not too keen on adding 'doctest:' everywhere though. Some of us
type these things often.

Nathann

On 29 October 2015 at 13:51, Vincent Delecroix

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "sage-devel" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/sage-devel/wH2JiJcA-8M/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

[John H Palmieri]

How important is it that the user see any of these? Should our documentation build hide these labels altogether? Should it hide "TESTS" blocks? Same questions when you ask for interactive help via "foo?".

  John

[Nathann Cohen]

> How important is it that the user see any of these? Should our documentation
> build hide these labels altogether? Should it hide "TESTS" blocks? Same
> questions when you ask for interactive help via "foo?".

This has been asked in different forms on this forum. I don't think
that there was any of us who disagreed with either hiding the "TEST"
blocks or folding them somehow in the HTML doc.

I think that it is more of a technical issue: who knows how to tweak
Sphinx to do that? Or is there any other way to do it outside of
sphinx?

Nathann

[John H Palmieri]

Since we can already detect doctests, and among those we can already detect the various doctest flags, we should be able to hide those flags. The TESTS blocks are a different situation. If we insist that TESTS blocks are always the last thing in the docstring, I can imagine it working.

--
John

[Nathann Cohen]

Hello,

> Since we can already detect doctests, and among those we can already detect
> the various doctest flags, we should be able to hide those flags. The TESTS
> blocks are a different situation. If we insist that TESTS blocks are always
> the last thing in the docstring, I can imagine it working.

There are cases where this would be wrong: I saw some code whose
'references' appeared after a test block. However, that should not be
very hard to check manually, so that we could make sure that we first
fix all such occurrences.

While I don't have much technical knowledge of our testing mechanisms,
I would be tempted to say that the way we filter the doctests is with
some 'ugly grep'.

This, to say that I am not sure that we currently have any notion of
'example block' or 'example environment'.

Additionally, this filtering is done through introspection (not sure,
but that would explain why we can't doctest cdef functions). I.e. the
doctest code loeds all functions and parses its content.

In order to only display a reduced version of the docstring to the
user we would have to parse and change the docstring when the function
is defined, or perhaps (better) when the user asks for the doc (though
I don't know how that would work at the moment). In simpler terms, we
need to insert some program between the doctest and the user who wants
to read it. Which is not something that the doctests scripts do.

I should have said sufficiently many erroneous things to let everybody
who's willing come and dispute me on specific points. I should stop
here.

Perhaps I should add, just in case, that I also believe that this
would be a nice addition. The reason why we have 'tests' blocks is
that some of our tests are not meant to be understood by users, or
might even scare them if they saw the complicated codes.

Nathann

[William Stein]

Yep! That's why I called it "not tested". In my opinion, the only
reasonable change would be

" # not tested" ----> " # broken"

That would probably create an even more accurate impression.

-- William

>
>
> --
> You received this message because you are subscribed to the Google Groups
> "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

--
William (http://wstein.org)

[Nathann Cohen]

> Yep! That's why I called it "not tested". In my opinion, the only
> reasonable change would be
>
> " # not tested" ----> " # broken"
>
> That would probably create an even more accurate impression.

I flagged many doctests with 'not tested' because running them would
take 10+s per function (and there are many of them). Those functions
take no input whatsoever, and are just meant to return a big object
(examples in graphs/strongly_regular_db).

The same happens for some doctests that rely on optional LP solvers:
they can take <1s with CPLEX/Gurobi, but 'standard sage' (with GLPK)
cannot be expected to ever solve them.

I also see calls to 'graph_editor' flagged as 'not tested' in the
graph/ folder, but as I do not know this code I cannot tell the
reason.

Nathann

[William Stein]

On Thu, Oct 29, 2015 at 10:12 AM, Nathann Cohen <nathan...@gmail.com> wrote:
>> Yep! That's why I called it "not tested". In my opinion, the only
>> reasonable change would be
>>
>> " # not tested" ----> " # broken"
>>
>> That would probably create an even more accurate impression.
>
> I flagged many doctests with 'not tested' because running them would
> take 10+s per function (and there are many of them). Those functions
> take no input whatsoever, and are just meant to return a big object
> (examples in graphs/strongly_regular_db).

That's what

# long time

is supposed to be for.

>
> The same happens for some doctests that rely on optional LP solvers:
> they can take <1s with CPLEX/Gurobi, but 'standard sage' (with GLPK)
> cannot be expected to ever solve them.

So if a user tries to use them they will be effectively broken?

> I also see calls to 'graph_editor' flagged as 'not tested' in the
> graph/ folder, but as I do not know this code I cannot tell the
> reason.

It's a graphical editor that runs in a web browser. (Or at least it
was 7 years ago...)

>
> Nathann

[Jori Mäntysalo]

On Thu, 29 Oct 2015, John H Palmieri wrote:

> Should it hide "TESTS" blocks? Same questions when you ask for
> interactive help via "foo?".

foo??? should do what foo?? does now, i.e. show the source. foo?? should
do what foo? does now, and foo? should hide TESTS, ALGORITHM and AUTHOR.

--
Jori Mäntysalo

[John H Palmieri]

There are some doctests labeled "not tested" because they produce a new git branch or some other unwanted result.

--
John

[William Stein]

That would be incompatible and inconsistent with IPython, which is
used (very) widely outside of Sage, and also *is* the sage command
line.

-- William


--
William (http://wstein.org)

[Jori Mäntysalo]

On Thu, 29 Oct 2015, William Stein wrote:

>> The same happens for some doctests that rely on optional LP solvers:
>> they can take <1s with CPLEX/Gurobi, but 'standard sage' (with GLPK)
>> cannot be expected to ever solve them.

> So if a user tries to use them they will be effectively broken?

But do we have "# optional - cplex OR gurobi"? AFAIK no. And to the user
it seems stupid to have

EXAMPLES:

First we try with CPLEX::

sage: x = do_something(42, None) # optional - cplex
'foo'

And now with Gurobi::

sage: x = do_something(42, None) # optional - gurobi
'foo'

--
Jori Mäntysalo

[Jori Mäntysalo]

On Thu, 29 Oct 2015, William Stein wrote:

>> foo??? should do what foo?? does now, i.e. show the source. foo?? should do
>> what foo? does now, and foo? should hide TESTS, ALGORITHM and AUTHOR.

> That would be incompatible and inconsistent with IPython - -

OK. Then maybe foo? , foo?+ and foo?? ?

--
Jori Mäntysalo

[Nathann Cohen]

> That's what
>
> # long time
>
> is supposed to be for.

I don't think so. You wouldn't want to see a doctest take one hour to
run, even if it is flagged with 'long time'. I don't know about you,
but I run 'long' doctests several time per day on some files.

>> The same happens for some doctests that rely on optional LP solvers:
>> they can take <1s with CPLEX/Gurobi, but 'standard sage' (with GLPK)
>> cannot be expected to ever solve them.
>
> So if a user tries to use them they will be effectively broken?

Broken? Why? They do return the expected output, they only do it very
very slowly (and perhaps not at all) when you don't have "one of
several solvers" installed. When you do, it can be instantaneous.
Thus, it is probably better to not test them every time somebody types
'make ptestlong'.

Nathann

[William Stein]

There is no place for tests that cannot be automatically run somehow
in our test suite. Absolutely any instance of this whatsoever is a
shortcoming in our test suite that we should *plan* to find a way to
address. (I'm of course not saying we should delete/change everything
right now.)

If you have tests that take too long, then we need another tag "# really long".
If there are tests that create git branches or have other "undesirable
side effects", then get better at writing tests that properly run in a
sandbox that addresses such side effects.

[Nathann Cohen]

> There is no place for tests that cannot be automatically run somehow
> in our test suite.

O_o

Since when? Says who? Surely you care about the pedagogical of doctests?

This graph [1] is a counterexample to a conjecture. It was expected to
be hamiltonian, and it is not. In the doctest, I says that
'g.is_hamiltonian()' returns False. That's one of the most interesting
things that there is to say about this graph, and Sage can solve it.

In less than a second with CPLEX, and in >2minutes (still running) with GLPK.

> Absolutely any instance of this whatsoever is a
> shortcoming in our test suite that we should *plan* to find a way to
> address. (I'm of course not saying we should delete/change everything
> right now.)
>
> If you have tests that take too long, then we need another tag "# really long".
> If there are tests that create git branches or have other "undesirable
> side effects", then get better at writing tests that properly run in a
> sandbox that addresses such side effects.

'get better'?....

I think you need to remember that all of us here are free workers. You
are CEO of SageMath, but that's another company. If you are not
satisfied with the doctesting framework, come show us that you still
know how to write python code.

Nathann

[1] http://doc.sagemath.org/html/en/reference/graphs/sage/graphs/graph_generators.html#sage.graphs.graph_generators.GraphGenerators.EllinghamHorton78Graph

[William Stein]

On Thu, Oct 29, 2015 at 11:03 AM, Nathann Cohen <nathan...@gmail.com> wrote:
> [...] come show us that you still know how to write python code.

Flame bait.

https://groups.google.com/forum/#!forum/sage-flame
http://wiki.sagemath.org/CodeOfConduct

--
William (http://wstein.org)

[Jori Mäntysalo]

Another thing about TESTS and EXAMPLES. Many functions returns
semantically a set but technically a list. Neighbours of a vertex in a
graph is an example. There is nothing wrong for these to return for
example [1,2] on PC and [2,1] on a Mac.

Now, to give a testable example of this kind of function one must put
sorted() to output or do someting like (1 in result, 2 in result) -->
True.

Should we show them to user? I think that this could be argued to both
directions.

--
Jori Mäntysalo

[Nathann Cohen]

Yo,

> Now, to give a testable example of this kind of function one must put
> sorted() to output or do someting like (1 in result, 2 in result) --> True.
>
> Should we show them to user? I think that this could be argued to both
> directions.

In those cases I'd say that '# random' does the job. The users may not
understand it though. Perhaps a 'platform-dependent' doctest or
something? That would behave exactly like 'random' but be less scary
to the users, who in this case wouldn't figure out why exactly this
command is 'random'.

'platform-dependent' is very long though... ^^;

Nathann

[Jeroen Demeyer]

Nothing prevents you from writing

# random (platform-dependent)

[Nathann Cohen]

> Nothing prevents you from writing
>
> # random (platform-dependent)

HMmmm.. Right, makes sense.

Nathann

[Jeroen Demeyer]

On 2015-10-29 18:30, William Stein wrote:
> That would be incompatible and inconsistent with IPython, which is
> used (very) widely outside of Sage, and also *is* the sage command
> line.

I agree with William. We should stay as compatible with IPython as we
can (which also implies that we should push some Sage-specific things to
IPython, maybe a project for SD70).

[John H Palmieri]

On Thursday, October 29, 2015 at 10:07:33 AM UTC-7, Nathann Cohen wrote:

Hello,

> Since we can already detect doctests, and among those we can already detect
> the various doctest flags, we should be able to hide those flags. The TESTS
> blocks are a different situation. If we insist that TESTS blocks are always
> the last thing in the docstring, I can imagine it working.

There are cases where this would be wrong: I saw some code whose
'references' appeared after a test block. However, that should not be
very hard to check manually, so that we could make sure that we first
fix all such occurrences.

Okay, http://trac.sagemath.org/ticket/19503 skips TESTS blocks when doing introspection ("sage: foo?"). It optionally skips them when building the reference manual, but I think that the default should be to include them: the reference manual should err toward having too much rather than too little.

I haven't touched the doctesting flags like "not tested". Even though they are brief and might be confusing, I could imagine cases where their lack could be even more confusing. We should instead add something to the tutorial about how to interpret our doctests.

--
John

[Jori Mäntysalo]

On Thu, 29 Oct 2015, John H Palmieri wrote:

> Since we can already detect doctests, and among those we can already detect
> the various doctest flags, we should be able to hide those flags. The TESTS
> blocks are a different situation. If we insist that TESTS blocks are always
> the last thing in the docstring, I can imagine it working.

Stupid question, but is there any list of possible blocks? (I.e. how well
structured our documentation is?) Artificial example: if I add functions
doing some numerical computation, can I just add

NUMERICAL STABILITY::

-blocks?

--
Jori Mäntysalo

[Jori Mäntysalo]

On Thu, 29 Oct 2015, Nathann Cohen wrote:

>> Now, to give a testable example of this kind of function one must put
>> sorted() to output or do someting like (1 in result, 2 in result) --> True.
>>
>> Should we show them to user? I think that this could be argued to both
>> directions.

> In those cases I'd say that '# random' does the job. The users may not
> understand it though. Perhaps a 'platform-dependent' doctest or
> something?

Hmm... yes, we could have "# random order". But then the example is not
really testable, i.e. it just tests if the code does not stuck. So which
one we prefer:

- Be nice to user with # random order

- Be better developer with sort(result) or something similar

- Make extra work and have both, one as an example and another as test.

- Make much more complicated doctest framework with some non-user-visible
additions like "# sort" that will be used in a test but not shown.

--
Jori Mäntysalo

[Jeroen Demeyer]

On 2015-10-30 08:35, Jori Mäntysalo wrote:
> - Be better developer with sort(result) or something similar

This option has my vote because it's the most explicit. We shouldn't
allow the doctest framework to behave too much different from an
interactive session, so I'm not in favour of behind-the-scenes sorting.

Jeroen.

[kcrisman]

>> That's what
>>
>>     # long time
>>
>> is supposed to be for.
>
> I don't think so. You wouldn't want to see a doctest take one hour to
> run, even if it is flagged with 'long time'. I don't know about you,
> but I run 'long' doctests several time per day on some files.
> 

If you have tests that take too long, then we need another tag "# really long".
 

Maybe we really do, since I can remember a number of tickets where tests were removed or changed from the most useful versions because even the "long" tests were running more than X seconds in the file (for a smallish value of X, not 3600 or something).  Then at least one could run the really long tests before a stable release.  But many of those really do have great value for users even if it is impractical to run them every time tests are run. 

[Jori Mäntysalo]

On Mon, 2 Nov 2015, kcrisman wrote:

>> If you have tests that take too long, then we need another tag "#
>> really long".

> Maybe we really do, since I can remember a number of tickets where tests
> were removed or changed from the most useful versions because even the
> "long" tests were running more than X seconds in the file (for a
> smallish value of X, not 3600 or something).  Then at least one could
> run the really long tests before a stable release.  But many of those
> really do have great value for users even if it is impractical to run
> them every time tests are run. 

I hope that Nathann's SRGs will be tested once a year, just to be sure.
And I definitely do not want to test them every time I run tests. (As I
assume that there will be more to come.)

Maybe two years ago I ran some tests to Singular's multivariate polynomial
factoring. IIRC sometimes it took 15 minutes to found a heisenbug. So yes,
there might be a place for # really long.

--
Jori Mäntysalo

[Nathann Cohen]

> Maybe we really do, since I can remember a number of tickets where tests
> were removed or changed from the most useful versions because even the
> "long" tests were running more than X seconds in the file (for a smallish
> value of X, not 3600 or something). Then at least one could run the really
> long tests before a stable release. But many of those really do have great
> value for users even if it is impractical to run them every time tests are
> run.

Hmmmmm.... It was once suggested (maybe by Jeroen?) that we could
have, for free, a decorator like that. Something like '# optional -
veryverylong'. Just add whatever tag you want after an 'optional' and
it will work.

There are some tests that I run 'from time to time', especially in a
development phase (while a big module is being written) to make sure
that all results stay correct. To take the example of Strongly Regular
Graphs (SRG) that Jori mentionned, I run from time to time a script
that builds them all up to n=2000 (the same was done on combinatorial
designs). That's something I run to double-check what I do, but I
would feel guilty if I forced anybody but me to run it regularly.

I don't know how such a type of test could be named, or whether this
name should be 'standard'. Just writing this to explain my situation,
in case it makes more sense to somebody else :-)

Nathann

[John Cremona]

On 2 November 2015 at 16:50, Nathann Cohen <nathan...@gmail.com> wrote:
>> Maybe we really do, since I can remember a number of tickets where tests
>> were removed or changed from the most useful versions because even the
>> "long" tests were running more than X seconds in the file (for a smallish
>> value of X, not 3600 or something). Then at least one could run the really
>> long tests before a stable release. But many of those really do have great
>> value for users even if it is impractical to run them every time tests are
>> run.
>
> Hmmmmm.... It was once suggested (maybe by Jeroen?) that we could
> have, for free, a decorator like that. Something like '# optional -
> veryverylong'. Just add whatever tag you want after an 'optional' and
> it will work.
>
> There are some tests that I run 'from time to time', especially in a
> development phase (while a big module is being written) to make sure
> that all results stay correct. To take the example of Strongly Regular
> Graphs (SRG) that Jori mentionned, I run from time to time a script
> that builds them all up to n=2000 (the same was done on combinatorial
> designs). That's something I run to double-check what I do, but I
> would feel guilty if I forced anybody but me to run it regularly.
>

I agree. There are test runs which take an hour (say, just for the
sake of argument) which I like to know still work -- or to see if
after some other changes they now only take half an hour -- but that
it much too long for a "long time" test.

John

> I don't know how such a type of test could be named, or whether this
> name should be 'standard'. Just writing this to explain my situation,
> in case it makes more sense to somebody else :-)
>
> Nathann
>

[Jori Mäntysalo]

On Tue, 3 Nov 2015, John Cremona wrote:

> I agree. There are test runs which take an hour (say, just for the
> sake of argument) which I like to know still work -- or to see if
> after some other changes they now only take half an hour -- but that
> it much too long for a "long time" test.

That will be a problem if you, for example, decide to become a reindeer
farmer, move to Lapland and stop making software. Local test files will
then be forgotten.

http://doc.sagemath.org/html/en/developer/coding_basics.html says "Skip
all doctests of a file/directory file: If one of the first 10 lines of a
file starts with any of r""" nodoctest - -". So can we just add those
extra tests to files marked with that?

--
Jori Mäntysalo

[Vincent Delecroix]

One solution with long doctest is to (optionally) flag them with a time
indication

# long 30sec
# long 1min
# long 1hour
# long 3year

Then, when testing we could add an option to skip the one which are too
long for our purpose

sage -t --all --skip-longer-than 2min

And I think that the default could reasonably be 10sec.

[Jeroen Demeyer]

On 2015-11-04 12:19, Vincent Delecroix wrote:
> One solution with long doctest is to (optionally) flag them with a time
> indication
>
> # long 30sec
> # long 1min
> # long 1hour
> # long 3year

That's actually a great idea!

Although, to simplify parsing, I would only allow seconds with a syntax
similar to optional:

# long time: 300s

[Vincent Delecroix]

I opened #19519 for that purpose.

[kcrisman]

That's actually a great idea!

Although, to simplify parsing, I would only allow seconds with a syntax
similar to optional:

# long time: 300s

Good resolution, very nice. 

[John Cremona]

+1

[William Stein]

Does this mean computers aren't constantly getting faster anymore? l miss the 90s... :-)

--

You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To post to this group, send email to sage-...@googlegroups.com.
Visit this group at http://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.

--
Sent from my massive iPhone 6 plus.

[Jeroen Demeyer]

On 2015-11-04 16:08, William Stein wrote:
> Does this mean computers aren't constantly getting faster anymore?

I think you have a point. They do get more cores and more SIMD
parallelization though.

[Jori Mäntysalo]

On Wed, 4 Nov 2015, William Stein wrote:

> Does this mean computers aren't constantly getting faster anymore? l miss
> the 90s... :-)

# long time - 90bogoseconds

?

--
Jori Mäntysalo

[Samuel Lelievre]

We could also have something like

    # test yearly

and just make sure to test those at least yearly.