Fix convert bug #320
Fix convert bug #320
Conversation
| @@ -17,7 +17,7 @@ convert(::Type{Interval{T}}, x::Bool) where {T} = convert(Interval{T}, Int(x)) | |||
| convert(::Type{Interval{T}}, x::Real) where {T} = atomic(Interval{T}, x) | |||
| convert(::Type{Interval{T}}, x::T) where {T<:Real} = Interval{T}(x) | |||
| convert(::Type{Interval{T}}, x::Interval{T}) where {T} = x | |||
| convert(::Type{Interval{T}}, x::Interval) where {T} = atomic(Interval{T}, x) | |||
| convert(::Type{Interval{T}}, x::Interval{S}) where {T,S} = atomic(Interval{T}, x) | |||
|
|
|||
| convert(::Type{Interval}, x::Real) = (T = typeof(float(x)); convert(Interval{T}, x)) | |||
| convert(::Type{Interval}, x::Interval) = x | |||
There was a problem hiding this comment.
Not directly related to the issue, but this definition is the same as the one on line 20 and should probably be deleted (especially since the result would not be correct if for some weird reason it ever gets called).
There was a problem hiding this comment.
Sorry, I don't understand which definition you're talking about that is the same as the one on line 20?
There was a problem hiding this comment.
On line 23:
convert(::Type{Interval}, x::Interval) = xSince Interval is the same as Interval{T} where {T} and nothing there requests both parameter to be the same.
| @@ -17,7 +17,7 @@ convert(::Type{Interval{T}}, x::Bool) where {T} = convert(Interval{T}, Int(x)) | |||
| convert(::Type{Interval{T}}, x::Real) where {T} = atomic(Interval{T}, x) | |||
| convert(::Type{Interval{T}}, x::T) where {T<:Real} = Interval{T}(x) | |||
| convert(::Type{Interval{T}}, x::Interval{T}) where {T} = x | |||
There was a problem hiding this comment.
| convert(::Type{Interval{T}}, x::Interval{T}) where {T} = x | |
| convert(::Type{Interval{T}}, x::Interval{T}) where {T<:Real} = x |
There was a problem hiding this comment.
While I see your point, this would make the code harder to maintain if we change the parameter type constrain.
There was a problem hiding this comment.
You can add a constant and use that instead of Real.
const IT = Real # type constraint of intervals| @@ -17,7 +17,7 @@ convert(::Type{Interval{T}}, x::Bool) where {T} = convert(Interval{T}, Int(x)) | |||
| convert(::Type{Interval{T}}, x::Real) where {T} = atomic(Interval{T}, x) | |||
| convert(::Type{Interval{T}}, x::T) where {T<:Real} = Interval{T}(x) | |||
| convert(::Type{Interval{T}}, x::Interval{T}) where {T} = x | |||
| convert(::Type{Interval{T}}, x::Interval) where {T} = atomic(Interval{T}, x) | |||
| convert(::Type{Interval{T}}, x::Interval{S}) where {T,S} = atomic(Interval{T}, x) | |||
There was a problem hiding this comment.
| convert(::Type{Interval{T}}, x::Interval{S}) where {T,S} = atomic(Interval{T}, x) | |
| convert(::Type{Interval{T}}, x::Interval) where {T<:Real} = atomic(Interval{T}, x) |
| @test m.sig == Tuple{typeof(convert),Type{Interval{T}},Interval{T}} where T | ||
|
|
||
| x = interval(1//1, 5//3) | ||
| @test convert(typeof(x), x) == x |
There was a problem hiding this comment.
| @test convert(typeof(x), x) == x | |
| @test convert(typeof(x), x) === x |
This would check that it does not create a new interval.
|
Tests are not passing (in Julia >= 1.2) due to Aside from this, I also interpreted Jeff's remark in JuliaLang/julia#32884 as restricting the parameter |
|
I see your point and wouldn't mind if it wasn't for #185: rewriting the type constrain here prevent the use of traits as a custom type allowed by a user would fail here. |
No description provided.