#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.