search

JuliaMath / Polynomials.jl

Polynomial manipulations in Julia
http://juliamath.github.io/Polynomials.jl/
Other
303 stars 75 forks source link

Conversions to and from RationalFunction broken #566

Open jishnub opened 5 months ago

jishnub commented 5 months ago

Julia nightly demands that convert(T, x) produce a result of type T. This is currently not the case with RationalPolynomials, where promote_rule at various places hard-codes Polynomial instead of using a proper promotion. As a consequence, the following is broken (and tests fail on Julia nightly in general):

julia> r = SparsePolynomial([1,2], :x)
(SparsePolynomial(1 + 2*x)

julia> rr = r // (r-1)
(1 + 2*x) // (2*x)

julia> oftype(rr, r)
ERROR: TypeError: in typeassert, expected RationalFunction{Int64, :x, SparsePolynomial{Int64, :x}}, got a value of type RationalFunction{Int64, :x, Polynomial{Int64, :x}}
Stacktrace:
 [1] oftype(x::RationalFunction{Int64, :x, SparsePolynomial{Int64, :x}}, y::SparsePolynomial{Int64, :x})
   @ Base ./essentials.jl:641
 [2] top-level scope
   @ REPL[12]:1
jverzani commented 5 months ago

Thanks for letting me know!

jishnub commented 5 months ago

Note that there are other instances like https://github.com/JuliaMath/Polynomials.jl/blob/263b965ba2b5c6cec7b48aa4d760658707fba1c4/src/rational-functions/common.jl#L52-L64 where the convert explicitly uses Polynomial, which may not be the correct type

jverzani commented 5 months ago

Yes, that one was deliberate. Perhaps it should just be deprecated and the user should convert to a polynomial type without negative powers before using.

Thanks again! I really appreciate your time and effort to point these out.