Skip to content

Phase out most usage of MatrixElem union #2255

@fingolfin

Description

@fingolfin

It is defined like this:

const MatrixElem{T} = Union{MatElem{T}, MatRingElem{T}}

Unfortunately the difference between MatrixElem and MatElem is highly confusing for people coming newly to OSCAR -- and also for more experience users and developers. More than once I've surprised people who've been working on OSCAR for quite some time by introducing them to the fact that these two types exist and are different. More than once we've seen code were someone by accident used MatrixElem, usually not even aware that it is something different than MatElem.

Now, I don't propose we remove this union, at least at this point (it's being used in too many places to make this practical). And it certainly has its uses.

But I'd really prefer if we could at least strip any use of it from documentation. So for example, if a docstring currently lists

    charpoly(Y::MatrixElem{T}) where {T <: RingElement}

then this could be changed to

    charpoly(Y::MatElem{T}) where {T <: RingElement}
    charpoly(Y::MatRingElem{T}) where {T <: RingElement}

and I think this would be more helpful to users.


Beyond that, I just search for MatrixElem in Oscar.jl, which gave 95 hits; I strongly suspect many of those really should be using MatElem (though of course it can be hard to tell, and to a degree, it's a matter of choice). But I'd argue that if some method really is meant to not just take plain matrices but also MatRingElem, then why stop there, why doesn't it also take MatrixGroupElem ?


BTW right next to this type, we also define

const PolynomialElem{T} = Union{PolyRingElem{T}, NCPolyRingElem{T}}

which has a somewhat similar problem, but it is hardly used at all (in fact I think only in AA, and there only really in src/Poly.jl (but through that it appears in docs, which I don't like much for the same reason as with MatrixElement). For this one, we should consider simply not exporting it anymore. And of course removing mentions in docstrings.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions