Default methods for `terms()` and `polynomial()` create an infinite recursion

[rdeits]

I just noticed that if you create a custom type MyPoly <: AbstractPolynomial subtype and do not implement polynomial() or terms(), then the default implementation of terms(p::MyPoly) calls this method: https://github.com/JuliaAlgebra/MultivariatePolynomials.jl/blob/82cafc7a0b55bd0b83e9ffa0940dabdce93b2e5a/src/polynomial.jl#L120 which in turn calls https://github.com/JuliaAlgebra/MultivariatePolynomials.jl/blob/82cafc7a0b55bd0b83e9ffa0940dabdce93b2e5a/src/polynomial.jl#L49 which just returns p. So we end up with terms(p) = terms(polynomial(p)) which just becomes terms(p) again and so on forever. This is particularly unfortunate because IJulia's display methods end up calling terms() which causes a stack overflow when trying to display a MyPoly.

My personal preference would be to change the API a bit so that instead of having terms(::AbstractPolynomial) try to be clever by calling polynomial(p), it should just fail early with a MethodError. Custom types would need to implement terms() themselves, but that doesn't seem so bad.

Alternatively, we could at least check in that terms() method if polynomial(p) === p and throw an error telling users they need to implement one or the other.

Any thoughts?

[blegat]

There are 4 types of AbstractPolynomialLike:

• AbstractTermLike
• AbstractPolynomial that implement terms (e.g. TypedPolynomials`)
• AbstractPolynomial that implement coefficients and monomials (e.g. DynamicPolynomials).
• polynomials that need to be transformed to an AbstractPolynomial if we want to get the list of terms (e.g MatPolynomial and SOSDecomposition. The method https://github.com/JuliaAlgebra/MultivariatePolynomials.jl/blob/82cafc7a0b55bd0b83e9ffa0940dabdce93b2e5a/src/polynomial.jl#L120 is useful for the first and last types. We could maybe distinguish the 2nd and 3rd types with subtypes of AbstractPolynomial and add a subtype of AbstractPolynomialLike for the 4th type. Another solution would be to use something a function like termiterator(::AbstractPolynomialLike) which returns either TermsIterator() (2nd type), CoeffMonoIterator() (3rd type) or NoIterator() (4th type). Differentiating the 2nd and 3rd category is something I have been thinking about for a long time. Currently most of the code in this package assume that the polynomial is in the 2nd category which is a bit inefficient if it is in the third, it is a bit like the differentiation between arrays that are fast for linear indexing and those who prefers cartesian indexing in base.

For this issue, a simplest solution would be to define terms(::AbstractPolynomial) returning a MethodError but in the long term, I would imagine that for the 3rd type, it should do something like https://github.com/JuliaAlgebra/DynamicPolynomials.jl/blob/6c12a0d856b9d739056ba64dd9fbbeac655b4293/src/poly.jl#L86-L98 instead of throwing a MethodError.