Remove a type restriction in Base.QuadGK.Segment (#19626)#19627
Remove a type restriction in Base.QuadGK.Segment (#19626)#19627stevengj merged 4 commits intoJuliaLang:masterfrom
Conversation
|
is what you're trying to accomplish testable without needing the rest of Unitful.jl? there are some tests for quadgk in |
|
I have an idea for how to do this; will get back to you once implemented. |
|
I added some tests. If you're wondering why I used a module instead of putting my additions into the quadgk test set, it is because I was running into some namespace issue that caused the tests to fail. I put my new definitions and tests into a module like done here and the tests pass now. |
test/math.jl
Outdated
|
|
||
| # Necessary with infinite or semi-infinite intervals since !(MockQuantity <: Real) | ||
| # and do_quadgk tests if eltype(s) <: Real. | ||
| function _do_quadgk{Tw,T<:Real}(f, s::Array{MockQuantity{T},1}, n, ::Type{Tw}, |
There was a problem hiding this comment.
This seems overly complicated. For testing this PR, it should be sufficient to have an f(x) where the integrand f is a MockQuantity, but x is still Float64.
There was a problem hiding this comment.
Agreed, I became overexuberant thinking about how I would do this in Unitful.
| Base.promote_rule{T,S}(::Type{MockQuantity{T}}, ::Type{MockQuantity{S}}) = | ||
| MockQuantity{promote_type(T,S)} | ||
| Base.convert{T}(::Type{MockQuantity{T}}, x::MockQuantity) = MockQuantity(T(x.val)) | ||
|
|
There was a problem hiding this comment.
You shouldn't need to define promotion rules and conversion for a minimal working example. Just define the minimum number of methods necessary for a test.
test/math.jl
Outdated
| using Base.Test | ||
| # Begin tests for 19626: Unitful compatibility. | ||
| # Define a mock physical quantity type | ||
| immutable MockQuantity{T} <: Number |
There was a problem hiding this comment.
It seems like it should work now even if it is not a subtype of Number?
There was a problem hiding this comment.
If it is not a subtype of Number then I get a failure in vecnorm.
|
I think I have the minimum number of definitions needed to get the test working now. Happy to implement further changes. |
test/math.jl
Outdated
| isless(a::MockQuantity, b::MockQuantity) = isless(a.val, b.val) | ||
|
|
||
| # isless defn. necessary so that default abstol plays nicely with MockQuantity | ||
| isless(y::Number, x::MockQuantity) = y == 0 ? isless(MockQuantity(0), x) : |
There was a problem hiding this comment.
I would just pass abstol = MockQuantity(0.0)
test/math.jl
Outdated
|
|
||
| # Define a mock physical quantity type | ||
| immutable MockQuantity{T} <: Number | ||
| val::T |
There was a problem hiding this comment.
Is the <: Number necessary? I wouldn't bother parameterizing the type. Just make val::Float64.
There was a problem hiding this comment.
<: Number is necessary, otherwise I get an error from vecnorm.
|
|
4 participants
See #19626