-
Notifications
You must be signed in to change notification settings - Fork 39
Generalise Zeros +/- #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generalise Zeros +/- #219
Conversation
Codecov Report
@@ Coverage Diff @@
## master #219 +/- ##
==========================================
- Coverage 99.08% 99.04% -0.05%
==========================================
Files 4 4
Lines 656 627 -29
==========================================
- Hits 650 621 -29
Misses 6 6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| return -b + a | ||
| end | ||
| -(a::AbstractRange, b::ZerosVector) = a + b | ||
| @inline elconvert(::Type{T}, A::AbstractRange) where T = T(first(A)):T(step(A)):T(last(A)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment that this is a work around for convert(AbstractArray{Float64}, 1:5) not returning a range.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be map(Float64, 1:5) instead? This does produce a range, although it's not guaranteed.
julia> map(Float64, 1:4)
1.0:1.0:4.0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a bit too "punny" so lets just leave this as is for now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that we already have _copy_oftype defined that does something similar to what elconvert does. It'll be good to merge the two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will have a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that we already have
_copy_oftypedefined that does something similar to whatelconvertdoes. It'll be good to merge the two.
It doesn't work since
julia> _copy_oftype(1:5,Float64)
5-element Vector{Float64}:
1.0
2.0
3.0
4.0
5.0JuliaArrays#219 broke compatibility with static arrays and thus downstream tests: ```julia Got exception outside of a @test LoadError: MethodError: +(::StaticArraysCore.SVector{2, Float64}, ::FillArrays.Fill{Float64, 1, Tuple{Base.OneTo{Int64}}}) is ambiguous. Candidates: +(a::AbstractArray, b::FillArrays.AbstractFill) in FillArrays at /home/runner/.julia/packages/FillArrays/VUZcr/src/fillalgebra.jl:242 +(a::StaticArraysCore.StaticArray, b::AbstractArray) in StaticArrays at /home/runner/.julia/packages/StaticArrays/4WE4t/src/linalg.jl:14 Possible fix, define +(::StaticArraysCore.StaticArray, ::FillArrays.AbstractFill) ``` https://github.com/SciML/OrdinaryDiffEq.jl/actions/runs/4444632992/jobs/7803006749?pr=1908#step:6:920 But it's not clear that the change to these operators was even necessary for anything.
Include a method
elconvertwhich could be overloaded by custom types to resolve, etc,Zeros(∞)+(1:∞).Some other +/- methods in
FillArrays.jlare moved tofillalgebra.jlResolve Ones{Bool}±Zeros{Bool} should be Ones{Int} #209