#14184: Fix # optional tags
----------------------------------+-----------------------------------------
       Reporter:  jdemeyer        |         Owner:  mvngu              
           Type:  defect          |        Status:  needs_work         
       Priority:  major           |     Milestone:  sage-5.8           
      Component:  doctest         |    Resolution:                     
       Keywords:                  |   Work issues:                     
Report Upstream:  N/A             |     Reviewers:  Karl-Dieter Crisman
        Authors:  Jeroen Demeyer  |     Merged in:                     
   Dependencies:                  |      Stopgaps:                     
----------------------------------+-----------------------------------------
Changes (by kcrisman):

  * status:  needs_review => needs_work
  * reviewer:  => Karl-Dieter Crisman


Comment:

 > This patch is mainly motivated by #12415, which has different (more
 strict) parsing for `# optional`, leading to less false positives (but
 also less true positives, hence this patch).
 I figured :)
 > >  * Did we change the `convert` ones to `ImageMagick`, or vice versa?
 Anyway, that should be standardized if it wasn't already elsewhere (this
 feels familiar)
 > I have no idea.
 I was right - we did change it to the program needed.
 `search_src('ImageMagick')` gets me a lot of hits, particularly in
 plot/animate.py
 {{{
 plot/animate.py:430:            sage: a.show()       # optional --
 ImageMagick
 plot/animate.py:435:            sage: a.show(iterations=3)    # optional
 -- ImageMagick
 plot/animate.py:439:            sage: a.show(delay=50)        # optional
 -- ImageMagick
 }}}
 I think this needs to be uniformized.  See #11170, where this was done,
 except apparently to the guys here (see also #10655 and #7981 for
 background).  By the way, is the `--` versus `-` important?
 > >  * I'm not sure why something can depend on dot2tex but not graphviz,
 and even more weirdly, graphviz is experimental, but not dot2tex ?!?!?
 Though I guess it is okay...
 > I have no idea what these packages are.
 It's ok, I think this is okay after looking at the various package lists.
 > >  * `somewhat random` - is that an official designation?
 > The doctester looks for "random", so it will be recognized. I think it's
 fine.
 Ok
 > >  * `< 1/100th of a second ` - well, I guess it might be!  I guess we
 can leave that.
 > This can be seen as "just a comment" without further meaning.
 Just wanted to make sure that additional comments were okay in doctest
 lines.
 > >  * What is the `time` command?  The one that we always have?  I'm
 confused here.
 > I have no idea.
 Got it - see the line above them (which your script didn't catch)?
 {{{
 sage: v, t = qsieve(n, time=True)   # uses the sieve    (optional: time
 doesn't work on cygwin)
 }}}
 This line should be changed to "look nice", and now we know why this was
 marked optional.  It hardly seems fair to make something optional that
 only doesn't work on a not-yet-supported platform... maybe we should
 remove these as optional?  I'll leave that to you.
 > >  * The `sage/lfunctions/sympow.py` test seems ... odd.  Should it
 instead be long test?  Note that the previous command just says optional,
 with no indication of why.
 > True, it's odd. I think it means that you need to run `sympow` first to
 create some tables, but I don't know more.
 I believe you are right.  But the doctest makes no sense as an automatic
 doctest in that case.  See [http://hg.sagemath.org/sage-
 main/file/5714ed3eab6a/sage/lfunctions/sympow.py#l116 the relevant note].
 In fact, I get the bizarre (newlines not rendered)
 {{{
 sage: a = sympow.L(EllipticCurve('11a'), 2, 16); a
 "Inertia Group is  C1 MULTIPLICATIVE REDUCTION\nConductor is 11\n**ERROR**
 P02L not found in param_data file\nIt can be added with './sympow
 -new_data 2'"
 }}}
 So I believe that both of them should probably be made `optional -
 precomputations`, or perhaps even better to add
 {{{
 sage: sympow('-new_data 2') # not tested
 }}}
 to the doctest and make them all not tested, since by definition someone
 would have to do something "by hand" to make them testable.  Or do you
 have another creative solution?  I can think of a couple, but they are
 annoying and probably pointless.

 There should also be better error catching there, but I figure anyone
 using sympow knows what they are doing enough to read the documentation -
 or is that worth a ticket as well?

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14184#comment:11>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica, 
and MATLAB

-- 
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to