Fix Stackoverflow when creating interval of intervals
Fix #182. I have define a new type alias of all supported type (basically all built-in number type). Therefore this make the implementation the less generic possible, which is also the safest.
The change also somehow confuse Julia when it comes to finding the most specific method in some cases, for unknown reason.
Finally a small change in the display function to be sure to avoid an infinite recursion by using specifically the BigFloat constructor.
Coverage remained the same at 92.444% when pulling 43a10732e226d0222f4cb27fb3b905e5193fa0a9 on Kolaru:fix_display_recursion into cab2433d6719899c38c0c3514fc935208614bdd0 on JuliaIntervals:master.
Coverage decreased (-0.07%) to 90.958% when pulling 337ce67cfa35fd1d95ac68c75a6ebb4b2c19d084 on Kolaru:fix_display_recursion into 4d0eba52b8f8cd883f7b6c9b3f4cd06c4c22d0b7 on JuliaIntervals:master.
Codecov Report
:exclamation: No coverage uploaded for pull request base (
master@1516c5e). Click here to learn what that means. The diff coverage is88.88%.
@@ Coverage Diff @@
## master #185 +/- ##
=========================================
Coverage ? 82.31%
=========================================
Files ? 25
Lines ? 1227
Branches ? 0
=========================================
Hits ? 1010
Misses ? 217
Partials ? 0
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/display.jl | 80.61% <ø> (ø) |
|
| src/intervals/arithmetic.jl | 94.11% <ø> (ø) |
|
| src/intervals/intervals.jl | 87.23% <88.88%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 1516c5e...b4fbd97. Read the comment docs.
Thanks @Kolaru for looking into this. I just checked in Julia 1.0 and the problem reported in #182 persists.
I like your idea of introducing IASupportedType here. However, your approach may be too restrictive, in the sense that a user may introduce another struct which is subtype of Real, for which it makes sense to define intervals; an example could be DoubleDouble or ArbFloat.
I think the following would allow for such an enlargement, but I haven't being able to test it in Julia 1.0.
using InteractiveUtils: subtypes
const IASupportedType = Union{setdiff(subtypes(Real), [AbstractInterval])...}
This would result on IASupportedType being Union{AbstractIrrational, AbstractFloat, Integer, Rational}, which is essentially what you listed.
Another option, as @dpsanders suggested, is that Interval(::Interval) throws an error.
What about a trait-like solution ? Something like
struct Interval{T} where {T <: Real}
a::T
b::T
Interval{T}(::Type{Val{true}}, a::T, b::T) = new(a, b)
Interval{T}(::Type{Val{false}}, ::T, ::T) = error("Type $T not good for interval arithmetic.")
end
Interval(a::T, b::T) where {T <: Real} = Interval(Val{isGoodForIA(T)}, a, b)
So to make a custom type A usable in interval, the user would simply have to write
isGoodForIA(A) = true
Like that absurd intervals are never passed silently, but it can still easily be extended with arbitrary type. Also I don't think this would justifiy adding SimpleTraits.jl as dependency, since it is quite simple still.
I closed the wrong PR, sorry for this.
What's the status here? Should we merge this?
I need to go back to it to address your last review. I don't remember why I didn't do it earlier, but I'll do it shortly.
The coverage decrease comes from the fact that can_bound_interval is not found to be hit by Coveralls, probably because the method is always optimized out by the compiler. I don't think that makes much sense to test it directly, so this PR should be ready.
This is ready (and may also help avoid some amiguity error I get in #271 when trying to have generic constructors).
Codecov Report
Merging #185 (09a4036) into master (a3dc27f) will decrease coverage by
0.02%. The diff coverage is88.23%.
@@ Coverage Diff @@
## master #185 +/- ##
==========================================
- Coverage 91.55% 91.52% -0.03%
==========================================
Files 25 25
Lines 1752 1758 +6
==========================================
+ Hits 1604 1609 +5
- Misses 148 149 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/intervals/arithmetic.jl | 97.20% <50.00%> (ø) |
|
| src/intervals/intervals.jl | 91.42% <92.30%> (-0.76%) |
:arrow_down: |
| src/IntervalArithmetic.jl | 100.00% <100.00%> (ø) |
|
| src/display.jl | 88.81% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update a3dc27f...09a4036. Read the comment docs.