Opened 7 years ago
Closed 6 years ago
#16228 closed defect (fixed)
make tachyon respect standard verbosity settings
Reported by:  niles  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  graphics  Keywords:  tachyon, verbose, beginner 
Cc:  Merged in:  
Authors:  Paul Graham  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  0d85789 (Commits)  Commit:  0d857894ac1af964695d3e082d6fdc3bc46bfc67 
Dependencies:  Stopgaps: 
Description
Tachyon.save
and Tachyon.show
have their own arguments for verbose output and do not pay attention to the verbosity levels set by sage.misc.misc.set_verbose
. This makes for inconsistent and confusing interface.
Instead of checking the value of an optional argument, these methods should check sage's verbosity setting. This is done implicitly when messages are printed with sage.misc.misc.verbose
. An explanation of this change should be added to the documentation.
Change History (15)
comment:1 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:2 Changed 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:3 Changed 6 years ago by
comment:4 Changed 6 years ago by
I'd not completely agree with the task as set out in the ticket description. Namely, it should be OK to use optional arguments to override the Sage's verbosity setting. This way, Tachyon.show/save should check their verbosity arguments, and if none are give, pass to tachyon_rt
Sage's verbosity. Otherwise, they should pass the value of the optional argument given.
comment:5 Changed 6 years ago by
 Branch set to u/paulgraham5/make_tachyon_respect_standard_verbosity_settings
comment:6 Changed 6 years ago by
 Commit set to 8246af4a1ae7ee9fa6656ba0f7fa8e39bc7daa29
 Status changed from new to needs_review
New commits:
8246af4  Made tachyon use the standard verbosity setting as default.

comment:7 followup: ↓ 9 Changed 6 years ago by
Can you provide doctests illustrating the effects of setting sage.misc.misc.set_verbose
?
(don't forget to set sage.misc.misc.set_verbose
back to its original value in such a doctest)
comment:8 Changed 6 years ago by
comment:9 in reply to: ↑ 7 ; followup: ↓ 10 Changed 6 years ago by
Replying to dimpase:
Can you provide doctests illustrating the effects of setting
sage.misc.misc.set_verbose
?(don't forget to set
sage.misc.misc.set_verbose
back to its original value in such a doctest)
Okay. How do you normally go about creating doctests? I haven’t done that before. From reading http://www.sagemath.org/doc/developer/doctesting.html#beyondthesagelibrary , i gather that, like in the examples section of a function definition eg:
r""" Creates a PNG file of the scene. EXAMPLES:: sage: q = Tachyon() sage: q.light((1,1,10), 1,(1,1,1)) sage: q.texture('s') sage: q.sphere((0,0,0),1,'s') sage: q.show(verbose=2) """
I put these further examples in this section, then run $ ./sage t src/sage/plot/plot3d/tachyon.py ? Or Are you referring to another method? :)
comment:10 in reply to: ↑ 9 Changed 6 years ago by
Replying to paulgraham5:
I put these further examples in this section, then run $ ./sage t src/sage/plot/plot3d/tachyon.py ?
Right :)
Also, build the documentation and check that it looks good in the reference manual.
comment:11 Changed 6 years ago by
 Commit changed from 8246af4a1ae7ee9fa6656ba0f7fa8e39bc7daa29 to 50f9a14761c6b74b7dca95f3048b0d108f2163b3
Branch pushed to git repo; I updated commit sha1. New commits:
50f9a14  Introduced doctests to illustrate effects of changing global verbosity.

comment:12 Changed 6 years ago by
The reference manual looks okay, and the tests go through okay on my end. The doctests are a new concept to me, so hopefully they aren’t too bad :) .
comment:13 Changed 6 years ago by
 Branch changed from u/paulgraham5/make_tachyon_respect_standard_verbosity_settings to public/ticket/16228
 Commit changed from 50f9a14761c6b74b7dca95f3048b0d108f2163b3 to 0d857894ac1af964695d3e082d6fdc3bc46bfc67
comment:14 Changed 6 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
looks good to me.
comment:15 Changed 6 years ago by
 Branch changed from public/ticket/16228 to 0d857894ac1af964695d3e082d6fdc3bc46bfc67
 Resolution set to fixed
 Status changed from positive_review to closed
Since
Tachyon.show
callsTachyon.save
shouldTachyon.save
check the verbosity level and pass it totachyon_rt
insage.interfaces.tachyon
? or should all references to the verbosity level be removed fromTachyon.show
andTachyon.save
and just check the verbosity level intachyon_rt
?