Rewrite display.jl #210
Rewrite display.jl #210
Conversation
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #210 +/- ##
==========================================
- Coverage 90.45% 90.28% -0.18%
==========================================
Files 23 23
Lines 985 988 +3
==========================================
+ Hits 891 892 +1
- Misses 94 96 +2
Continue to review full report at Codecov.
|
1 similar comment
src/display.jl
Outdated
| @@ -23,8 +23,13 @@ The following options are available: | |||
| - `sigfigs`: number of significant figures to show in `standard` mode | |||
There was a problem hiding this comment.
I think it is worth writing here that the default number is 6.
src/display.jl
Outdated
| """ | ||
| function displaymode(;decorations=nothing, format=nothing, sigfigs=-1) | ||
| function setdisplay(format=nothing; decorations=nothing, sigfigs::Integer=-1) |
There was a problem hiding this comment.
I guess there is a reason why the default is nothing for each kw, but why is this better than setting them directly to the values specified in display_params?
There was a problem hiding this comment.
That's a good idea, thanks.
src/display.jl
Outdated
|
|
||
| else | ||
| throw(ArgumentError("`decorations` must be `true` or `false`")) | ||
| end | ||
| end | ||
|
|
||
| if sigfigs >= 0 |
There was a problem hiding this comment.
What is the meaning of sigfigs=0? Anywhay, I get the following:
julia> @interval(0.1, 0.2)
[0.0999999, 0.200001]
julia> setdisplay(:standard, sigfigs=0)
0
julia> @interval(0.1, 0.2)
[0.09, 0.3]There was a problem hiding this comment.
I have made 1 the minimum value of sigfigs.
| @@ -133,6 +146,9 @@ function subscriptify(n::Int) | |||
| end | |||
|
|
|||
|
|
|||
| # fall-back: | |||
There was a problem hiding this comment.
Why do you need basic_representation and representation, or isn't it enough with one function and several methods?
There was a problem hiding this comment.
I had just representation before. The problem is that representation(x::Interval{BigFloat}) calls representation for the interval part and then adds the part with the number of digits.
To do so, I had to use invoke, which is ugly. The simple solution was to add basic_representation instead.
|
I have made the changes you suggested, thanks. I'll merge once it goes green. |
1 similar comment
Fixes #202, #175, #131.
This changes the name of
displaymodetosetdisplay, and the syntax to the simplerinstead of the wordy