#7981: animate ignores options to show that are passed up from the plot command
----------------------------------------------------------------+-----------
Reporter: jason | Owner:
novoselt
Type: defect | Status:
positive_review
Priority: major | Milestone:
sage-4.6.1
Component: graphics | Keywords:
Author: Jason Grout, Andrey Novoseltsev | Upstream:
N/A
Reviewer: Tim Dumol, Marshall Hampton, Karl-Dieter Crisman | Merged:
Work_issues: |
----------------------------------------------------------------+-----------
Changes (by kcrisman):
* status: needs_review => positive_review
Comment:
Replying to [comment:14 novoselt]:
> Replying to [comment:13 kcrisman]:
> I think that it makes the syntax of save cleaner and easier to
understand (and document for that matter). As was recently mentioned on
sage-devel, one should use common sense when deciding whether to deprecate
something or change immediately, I think that these changes fall into the
latter category ;-)
Yeah, these two make sense. It looks like dpi will still work, given
`SHOW_OPTIONS`.
>As for `*args` I just think that it is a bad practice to call functions
with 20 or so possible parameters listing them without names.
Okay, I see what's going on here now. Especially since the order would be
open to suspicion!
> > As for `savenow`, it looks like with it being `False` we could still
create a Sage object. You are right that it seems a little redundant,
though!
>
> Isn't it a bug that `save` saves something in some cases when
`savenow=False`?..
No, just an undocumented feature! Since it doesn't save a ''graphic''. I
agree this seems odd, though, so not complaining.
> > Also, this needs a doctest (it's in the original patch) to show that
animate options actually work, at least in theory (if one looked at it and
ran the optional tests). So... needs work. Sorry :(
>
> I added the doctest. Was it the only reason for "needs work" or you
would like to have `save` parameters changed as well?
No, assuming this still applies by the buildbot, and since you've
explained the parameter issue fine, that's okay. The only reason I felt
justified in overruling the original positive review was because it didn't
actually include the doctest, though I had the other questions as well.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7981#comment:16>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
