Add "ignored" flag for doctests

[embray]

This changed the output checker to add an # ignored flag that can be used in doctests to run a line of a test but not check its output. Actually this is just a renaming of the existing # random flag, without the semantic weight of calling a result "random". "random" is still kept as an alias for "ignored" both for backwards compatibility, and because it could be useful as a semantic indication that some output truly is expected to be random.

This is useful for cases like the one I described here, where for documentation purposes it's nice to display the output of some code, but that output may not be reliable across platforms, and ultimately does not affect the result of the test.

CC: @nexttime

Component: doctest framework

Work Issues: coverage

Author: Erik Bray

Branch/Commit: u/embray/doctest-ignored @ 0a30f65

Reviewer: Julian Rüth

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

[embray]

Author: Erik Bray

[embray]

New commits:

0a30f65

Rename the 'random' marker to the more general 'ignored'.

[embray]

Commit: 0a30f65

[embray]

Branch: u/embray/doctest-ignored

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:3

In the description of ignored, I'd elaborate the use case you give (which isn't that obvious).

(Just about one sentence I'd say.)

---

Why did you add ?: to the ignored_marker regex???

[embray]

comment:4

Yeah, I could add a little more context to the example (it's a simplified version of the test that caused me trouble in the first place).

The ?: is just force of habit--I like to put it in () if I'm not actually planning to use the term in the parens as a match group. It's harmless either way.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:5

Replying to @embray:

The ?: is just force of habit--I like to put it in () if I'm not actually planning to use the term in the parens as a match group. It's harmless either way.

Ah, I see. So we won't (immediately) know whether ignored or random was specified if the pattern for these two markers matches, but don't care either, at least at the moment. That's ok.

(The other regexs aren't all optimal, e.g. regarding optional or parsing numbers in scientific notation, but that's for another ticket.)

[saraedum]

Reviewer: Julian Rüth

[saraedum]

comment:6

Looks good but the patchbot complains that "random" has no doctest. I guess you should add one though it is somewhat artificial.

[saraedum]

Work Issues: coverage

[jdemeyer]

comment:7

Revisiting this... To be honest, I never really liked the idea of having two different doctest markers doing the same thing. Wouldn't it be sufficient to use # random with additional comments to explain why it's random?

sage: 6*9  # random (depending on default number base)
42
sage: moon.phase  # random (depending on the phase of the moon)
'full'

[embray]

comment:8

The idea was to have #ignored eventually replace #random since the latter is poorly-named. It's useful to be able to skip individual doctests that are not "random".

[jdemeyer]

comment:9

Replying to @embray:

The idea was to have #ignored eventually replace #random since the latter is poorly-named.

I don't care much (it's bikeshedding anyway). However, if you do change # random to # ignored, I would prefer to actually make the changes in all doctests (not so hard using a regex) right now instead of "eventually". Having some doctests with # random and some with # ignored is confusing IMHO.

[embray]

comment:10

I don't know--I think it's useful to be able to say "this output is random". Depends on the case.

[jdemeyer]

comment:11

Replying to @embray:

I don't know--I think it's useful to be able to say "this output is random". Depends on the case.

I don't see much semantic difference between "random" and "ignored": both indicate that the output of the doctest is not always the same (but the test should be run to check that it does not raise exceptions). I would say: pick one and consistently use it.

[embray]

comment:12

Alright, I was just trying to preserve the original purpose of the flag, which I thought made some sense insofar as specifying that the output should be truly random, whereas there are other reasons to ignore the output of a test (maybe it's not reliably deterministic, but not random). Maybe having a separate #random would make more sense if it somehow checked that the output of that test were actually different between each run. But unless anyone actually needs that I'd be fine with getting rid of it entirely then.

[embray]

comment:13

On second thought, there's so many cases of # random in the source code that if I try to change it it may conflict with just about every other unmerged branch.

Perhaps it should suffice to just update the documentation to suggest using # ignored instead of # random and keep the latter deprecated.

[jdemeyer]

comment:14

Replying to @embray:

maybe it's not reliably deterministic, but not random

And what is the semantic difference between "not reliably deterministic" and "random"? I think you are trying to see a difference where there is none.

Again, I'm not strictly against this ticket, I just wouldn't bother...

[embray]

comment:15

In some cases it might not be random. For example, you might have a test that prints a dictionary, and the key order may be always the same on a particular Python version on a particular platform. But that doesn't mean it should be relied on to be ordered properly.

Or you might have a test that prints out a filesystem path that obviously won't be the same on all machines. That doesn't make it random. I just think random is a misnomer for this purpose. I don't think we're disagreement. You just say you wouldn't bother--but I already did because it bothered me. The only issue here is what to do with the existing name. I may have implemented something else but I don't think it's worth it to go changing the existing usage everywhere either :)