Document `(m)poly_type`, `(m)poly_ring_type` in the manual by JohnAAbbott · Pull Request #2314 · Nemocas/AbstractAlgebra.jl

JohnAAbbott · 2026-01-28T15:33:32Z

First attempt at tackling issue #1316
Feedback welcome

codecov · 2026-01-28T15:52:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.11%. Comparing base (1027804) to head (066d820).
⚠️ Report is 29 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2314      +/-   ##
==========================================
- Coverage   88.16%   88.11%   -0.05%     
==========================================
  Files         126      126              
  Lines       32047    32108      +61     
==========================================
+ Hits        28255    28293      +38     
- Misses       3792     3815      +23

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fingolfin · 2026-01-29T10:23:17Z

+```julia
+mpoly_type(::Type{T}) where T <: RingElement
+```


One problem with this approach (which unfortunately is used a lot in the AA docs) is that the documentation search does not pick up on it -- searching for mpoly_type will not find this.

This can be solved by using a ```@docs block instead, pulling in the docstring for mpoly_type. Of course we then can also edit and improve that docstring, drawing from the text you wrote below.

@JohnAAbbott ping did you see this?

Yes; I have just pushed a modification which I hope addresses your concern.

fingolfin · 2026-01-29T10:32:04Z

+This function is defined for generic polynomials and needs to be defined only for
+special polynomial rings, e.g. those defined by a C implementation.


This sentence is for people who develop new polynomial ring implementations; but not for regular users.

I think we need to make this difference very explicit to avoid confusion. But in this case, I would actually suggest to not put this into the docstring. Instead, this section here could look like this, roughly like this:

Any implementation of the multivariate polynomial ring interface needs to provide methods for the following two functions for all the rings it supports. ```docs mpoly_type mpoly_ring_type ``` The default methods for these use the `Generic.MPolyRing` types. But e.g. in Nemo a type `QQMPolyRing` is provided which has optimized multivariate polynomial rings over the rationals.

If we do decide to put this into the docstring, then I suggest we at least put it into a section called # Implementation, following the guidelines from the Julia documentation on "Writing Documentation", bullet point 10.

Now I am a little confused. What is the target readership for the documentation in mpoly_interface.md? At least from the first few paragraphs, it seems that the documentation is primarily for developers of new types of multivariate polynomial ring.
LINK https://nemocas.github.io/AbstractAlgebra.jl/dev/mpoly_interface/

Now I am a little confused. What is the target readership for the documentation in mpoly_interface.md? At least from the first few paragraphs, it seems that the documentation is primarily for developers of new types of multivariate polynomial ring.

As I explained during lunch today, the current documentation indeed suffers from not being clear on what the target audience are for

https://nemocas.github.io/AbstractAlgebra.jl/stable/mpolynomial/ versus

https://nemocas.github.io/AbstractAlgebra.jl/stable/mpoly_interface/

Indeed, the former right now seems to be a documentation for Generic.MPoly ?!
But really even as a user one needs to read both pages in order to be able to work. While as an implementor, even after reading both pages it is not super clear what one must implement; what one should implement (defaults are provided but most likely you can do better); and what one might optionally implement (for example factoring would be nice to have but is not a requirement of any of the interfaces).

As I explained, we ought to overhaul the documentation to improve this (and not just for mpoly). In my mind we want two or three pages:

one for "consumers" who use MPolys; this should describe the general interface

optionally: another "consumer" page describes anything truly specific to Generic.MPoly that end users ought to know about (similar pages could then exist for ZZMPolyRingElem etc. in Nemo)

one page for "implementors" as outlined above.

It would also be a good idea to keep the "four quadrants" of the Diátaxis system in mind.

I agree that the documentation seems to be "mixed up" as regards intended readership. This looks to be a good candidate for a separate issue -- and I suspect that reorganizing the documentation will be lengthy task, once a clear decision has been made on how it should be organized. In this PR I shall make only a small step in that direction.

Sure, untangling the docs is a separate issue.

But in the meantime this section is the only place for everyone to learn about these functions. So I still think we should use the docstring approach I outlined.

I have just read the website about the Diataxis system. It seems like a good starting point for a discussion about reorganizing the documentation -- I'm not entirely convinced by everything written there, but they do make some interesting points.

JohnAAbbott · 2026-02-03T13:53:09Z

Tangentially related: it may be my ignorance, but I don't understand why, in the examples for MyMPolyRing, the functions symbols and number_of_generators need to specify the type parameter T. Perhaps it is just for uniformity of presentation? Or may it is a Julia quirk?

thofma · 2026-02-03T13:59:24Z

Tangentially related: it may be my ignorance, but I don't understand why, in the examples for MyMPolyRing, the functions symbols and number_of_generators need to specify the type parameter T. Perhaps it is just for uniformity of presentation? Or may it is a Julia quirk?

Do you mean in the documentation? Can you point out the place?

JohnAAbbott · 2026-02-03T16:52:45Z

Tangentially related: it may be my ignorance, but I don't understand why, in the examples for MyMPolyRing, the functions symbols and number_of_generators need to specify the type parameter T. Perhaps it is just for uniformity of presentation? Or may it is a Julia quirk?

Do you mean in the documentation? Can you point out the place?

In the doc: docs/src/mpoly_interface.md near lines 130--140. The actual source code has only generic implementations: src/generic/MPoly.jl near lines 29--36.
Sorry, the arrival of your comment "flew under the radar".

thofma · 2026-02-03T17:26:52Z

I think you are right, the parameters are not necessary.

…ection to docstrings

JohnAAbbott · 2026-02-13T11:02:14Z

Some tests fail. The cause seems to be: in GenericTypes.jl (near line 312) we have

mutable struct NCPoly{T <: NCRingElem} <: AbstractAlgebra.NCPolyRingElem{T}

But it seems that we want it with T <: NCRingElement to allow some Julia types e.g. Rational{BigInt}. Am I correct?
But how did the tests work before my changes?

Ahhh! The extra definition in Poly.jl seems to be what does the trick:

poly_type(::Type{T}) where T<:RingElement = Generic.Poly{T}

But I do feel uneasy that the "main" definitions are in generic/NCPoly.jl then there is one extra definition in Poly.jl

JohnAAbbott · 2026-02-13T11:32:17Z

I am now confused about whether NCPoly{T} should work for T <: NCRingElem or for T <: NCRingElement.
This is a design issue, and should be addressed separately outside this PR.
The documentation and definitions of poly_type in file src/NCPoly.jl suggest it should work with T <: NCRingElement, but in practice it does not (since other code assumes T <: NCRingElem); so there is a "stray" definition of poly_type in file src/Poly.jl whose sole purpose appears to be patch up the situation -- this is not explained in Poly.jl.

JohnAAbbott · 2026-02-13T13:39:43Z

Hekp? Not sure how to fix the problem in Run tests / Documentation; I suppose it's easy, just I'm not sure what needs to be done.

thofma · 2026-02-13T14:13:02Z

You can click on the failing test. It is a link, which brings you to https://github.com/Nemocas/AbstractAlgebra.jl/actions/runs/21987838445/job/63526572062?pr=2314

thofma · 2026-02-13T14:33:32Z

The @docs block contains signatures, for which there does not exist a docstring.

JohnAAbbott · 2026-02-13T14:42:16Z

Thanks @thofma ! I had seen that, but was not sure what the approved way of proceeding is. The easiest (for me) would be to remove the signatures (inside the @docs block) which cause the complaints, but that would potentially mislead users by not informing them of part of the interface -- but that part is implemented in a "lazy/catch-all" way: in file src/NCPoly.jl near line 58

poly_type(x) = poly_type(typeof(x))

I did try inserting two new definitions:

poly_type(x::T) where T<:NCRingElement = poly_type(typeof(x))
poly_type(R::S) where S<:NCRing = poly_type(typeof(R))

but I did not create individual docstrings for them. Maybe the solution is to make a "universal" docstring for all functions called poly_type? But would that please the Julia doc demon?

thofma · 2026-02-13T14:49:50Z

Maybe the easiest solution is to do change the docstring to

@doc raw"""
    mpoly_type(::Type{T}) where T<:RingElement
    mpoly_type(::T) where T<:RingElement
    mpoly_type(::Type{S}) where S<:Ring
    mpoly_type(::S) where S<:Ring

Return the type of a (multivariate) polynomial whose coefficients have type `T` or
type `elem_type(S)`.
The type of the corresponding polynomial ring can be found via [`mpoly_ring_type`](@ref).

For univariate polynomials see [`poly_type`](@ref).

# Implementation

This function is already defined for generic multivariate polynomials (namely `Generic.MPoly{T}`),
so _needs to be defined only for_ special polynomial rings, _e.g._ those defined by
a C implementation.

# Examples
```jldoctest
julia> mpoly_type(AbstractAlgebra.ZZ(1))
AbstractAlgebra.Generic.MPoly{BigInt}

julia> mpoly_type(elem_type(AbstractAlgebra.ZZ))
AbstractAlgebra.Generic.MPoly{BigInt}

julia> mpoly_type(AbstractAlgebra.ZZ)
AbstractAlgebra.Generic.MPoly{BigInt}

julia> mpoly_type(typeof(AbstractAlgebra.ZZ))
AbstractAlgebra.Generic.MPoly{BigInt}
```
"""
mpoly_type

and then include it via

```@docs
mpoly_type
```

Similar for the other function.

fingolfin · 2026-02-13T15:47:37Z

Indeed: for a @docs block, what is visible in the end is the content of the docstrings it includes, not the list of identifiers insider the @docs block.

fingolfin · 2026-02-13T15:48:56Z

@@ -21,10 +21,17 @@ coefficient_ring(R::MPolyRing) = base_ring(R)
    mpoly_type(::Type{S}) where S<:Ring
    mpoly_type(::S) where S<:Ring


The user will see the text above this line, too!

Co-authored-by: Max Horn <max@quendi.de>

…pe, mpoly_ring_type

fingolfin · 2026-02-19T12:34:41Z


-See `src/generic/GenericTypes.jl` for an example of how to implement such a cache (which
-usually makes use of a dictionary).
+Implementaors of a new type of polynomial ring should see `src/generic/GenericTypes.jl`


Suggested change

Implementaors of a new type of polynomial ring should see `src/generic/GenericTypes.jl`

Implementors of a new type of polynomial ring should see `src/generic/GenericTypes.jl`

JohnAAbbott added 2 commits January 28, 2026 16:23

Added doc for poly_ring_type

d01b8ac

Added doc for mpoly_type, mpoly_ring_type

2bf424d

fingolfin reviewed Jan 29, 2026

View reviewed changes

fingolfin reviewed Feb 2, 2026

View reviewed changes

fingolfin added doc This change adds or pertains to documentation release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Feb 3, 2026

Changed julia tag into @docs; revised wording; added Implementation s…

0cb60dc

…ection to docstrings

Reactivate the "stray" defn of poly_type inside Poly.jl

0b9e3e7

JohnAAbbott marked this pull request as ready for review February 13, 2026 11:36

Added two "new" versions of poly_type, so that impl agrees with doc

899ea5d

fingolfin reviewed Feb 13, 2026

View reviewed changes

Comment thread docs/src/mpoly_interface.md Outdated

fingolfin reviewed Feb 13, 2026

View reviewed changes

Comment thread docs/src/poly_interface.md Outdated

fingolfin reviewed Feb 13, 2026

View reviewed changes

Comment thread docs/src/poly_interface.md Outdated

fingolfin reviewed Feb 13, 2026

View reviewed changes

JohnAAbbott and others added 3 commits February 13, 2026 17:01

Update docs/src/poly_interface.md

81cd94a

Co-authored-by: Max Horn <max@quendi.de>

Update docs/src/poly_interface.md

5e47073

Co-authored-by: Max Horn <max@quendi.de>

Changed to unified docstrings for poly_type, poly_ring_type, mpoly_ty…

e0dfe55

…pe, mpoly_ring_type

Removed 2 commented-out methods for poly_type

066d820

fingolfin reviewed Feb 19, 2026

View reviewed changes

fingolfin approved these changes Feb 19, 2026

View reviewed changes

fingolfin changed the title ~~Jaa/doc for poly ring type~~ Document (m)poly_type, (m)poly_ring_type in the manual Feb 19, 2026

fingolfin merged commit 17ba14e into Nemocas:master Feb 19, 2026
24 of 25 checks passed

		This function is defined for generic polynomials and needs to be defined only for
		special polynomial rings, e.g. those defined by a C implementation.

		@@ -21,10 +21,17 @@ coefficient_ring(R::MPolyRing) = base_ring(R)
		mpoly_type(::Type{S}) where S<:Ring
		mpoly_type(::S) where S<:Ring

	Implementaors of a new type of polynomial ring should see `src/generic/GenericTypes.jl`
	Implementors of a new type of polynomial ring should see `src/generic/GenericTypes.jl`

Conversation

JohnAAbbott commented Jan 28, 2026

Uh oh!

codecov Bot commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohnAAbbott commented Feb 3, 2026

Uh oh!

thofma commented Feb 3, 2026

Uh oh!

JohnAAbbott commented Feb 3, 2026

Uh oh!

thofma commented Feb 3, 2026

Uh oh!

JohnAAbbott commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohnAAbbott commented Feb 13, 2026

Uh oh!

JohnAAbbott commented Feb 13, 2026

Uh oh!

thofma commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thofma commented Feb 13, 2026

Uh oh!

JohnAAbbott commented Feb 13, 2026

Uh oh!

thofma commented Feb 13, 2026

Uh oh!

fingolfin commented Feb 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jan 28, 2026 •

edited

Loading

JohnAAbbott commented Feb 13, 2026 •

edited

Loading

thofma commented Feb 13, 2026 •

edited

Loading