Opened 8 years ago

Closed 8 years ago

#12507 closed task (fixed)

Mark random symbolic expression doctests with #random

Reported by: benjaminfjones Owned by: burcin
Priority: minor Milestone: sage-5.0
Component: symbolics Keywords: random, symbolic, doctest
Cc: burcin Merged in: sage-5.0.beta5
Authors: Benjamin Jones Reviewers: Burcin Erocal
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Three doctests in sage/symbolic/random_tests.py change expected value whenever a new symbolic function is added to Sage. In particular,

sage: [f for (one,f,arity) in _mk_full_functions()]
sage: random_expr(50, nvars=3, coeff_generator=CDF.random_element)
sage: random_expr(5, verbose=True)

This makes adding symbolic functions to the Sage library somewhat painful, especially if more than one such patch is contributed per Sage release.

The patch provided marks the three particular random tests using #random so that the doctest framework ignores the output from such tests.

Apply: trac_12507_v2.patch.

Attachments (2)

trac_12507.patch (1.7 KB) - added by benjaminfjones 8 years ago.
mark random symbolic doctest with #random tag
trac_12507_v2.patch (1.7 KB) - added by benjaminfjones 8 years ago.
Mark random symbolic doctest with #random tag; rebase to 5.0.beta4

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by benjaminfjones

mark random symbolic doctest with #random tag

comment:1 Changed 8 years ago by benjaminfjones

  • Type changed from PLEASE CHANGE to task

comment:2 Changed 8 years ago by burcin

  • Reviewers set to Burcin Erocal
  • Status changed from new to needs_review

comment:3 Changed 8 years ago by burcin

  • Description modified (diff)
  • Status changed from needs_review to positive_review

comment:4 follow-up: Changed 8 years ago by kcrisman

Hmm. While the second two are essentially 'random', I do have two questions.

  • How is # random usually used in our doctests? It seems like it's used when the answer is random, not when the answer is very non-random but changes with new functionality.
  • The first test is actually kind of neat, because it shows what we all have. Maybe it could be made non-random, but using a ... syntax? I know that fairly recently we even then would have had to change things, but at least that would slow the rate of needing to change that test down a lot, and still preserve it as a test checking that we weren't importing something really weird by mistake.

That said, the sentiment behind this is fine.


Does this depend on #9130?

Changed 8 years ago by benjaminfjones

Mark random symbolic doctest with #random tag; rebase to 5.0.beta4

comment:5 in reply to: ↑ 4 Changed 8 years ago by benjaminfjones

Replying to kcrisman:

Hmm. While the second two are essentially 'random', I do have two questions.

  • How is # random usually used in our doctests? It seems like it's used when the answer is random, not when the answer is very non-random but changes with new functionality.
  • The first test is actually kind of neat, because it shows what we all have. Maybe it could be made non-random, but using a ... syntax? I know that fairly recently we even then would have had to change things, but at least that would slow the rate of needing to change that test down a lot, and still preserve it as a test checking that we weren't importing something really weird by mistake.

That said, the sentiment behind this is fine.

It's my understanding that a doctest with a #random tag is run and runtime errors are reported, but the output of the test is not checked against the result listed in the docstring. I agree, the output of those "random" tests is not random, the same random seed is set at the start of the tests. I can't find the use of ... documented in the developer guide. In the guide the hash function is given as an example of a doctest that is marked #random because the output depends on the machine and the state of the python interpreter presumably. This doesn't mean the output of hash is random, just hard to determine at runtime.


Does this depend on #9130?

It doesn't depend on #9130, but I just rebased the patch to 5.0.beta4 because #4489 was merged and that changed the random tests. The patch trac_12507_v2.patch applies cleanly to 5.0.beta4 now.

comment:6 Changed 8 years ago by benjaminfjones

I meant #4498 in my last comment.

comment:7 Changed 8 years ago by jdemeyer

  • Description modified (diff)

Be sure to update the "apply" part of the description when needed

comment:8 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.0.beta5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.