Actually include docstrings into manual #317
Comments
|
On Mon, 21 Dec 2020 at 11:37, Max Horn ***@***.***> wrote:
Running documenter on the manual currently produces a bunch of warnings
about docstrings that are not included in the manual. We should probably
fix those.
┌ Warning: 18 docstrings not included in the manual:
│
│ Singular.permute_variables :: Tuple{spoly,Array{Int64,1},PolyRing}
This was added because someone wanted to use the corresponding Singular
function. We provide generic functionality for this via AbstractAlgebra,
under for example evaluation. It would make sense to implement such a
function in AbstractAlgebra generically though. But I'm uncertain if we
want to implement the interface like this. So this is very definitely
provisional from memory.
It was never officially documented for this reason.
│ Singular.modulo :: Tuple{smodule,smodule}
This looks like it could be related to functionality given a different
name in AbstractAlgebra, which might explain why it doesn't appear in the
docs. It's not precisely what we call a quotient module there, but it is
the equivalent for more general modules if I understand correctly.
│ Singular.Singular
I have no idea why this has a docstring.
│ AbstractAlgebra.Generic.kernel :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SIdAlgHom}}
│ AbstractAlgebra.Generic.kernel :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SAlgHom}}
Agreed, this should appear in the manual I think, though it is technically
already documented in AbstractAlgebra.
│ Singular.lift_std :: Tuple{smodule}
We are using names like blah_with_transform for functions of this kind in
our other packages. I suggest renaming it before including in docs.
│ Singular.eliminate :: Tuple{smodule,Vararg{spoly,N} where N}
Agreed it should be included.
│ Singular.R :: Union{Tuple{CxxWrap.CxxWrapCore.CxxPtr{Singular.libSingular.ideal},Val{:module}}, Tuple{T}} where T
│ Singular.R :: Union{Tuple{CxxWrap.CxxWrapCore.CxxPtr{Singular.libSingular.poly},Val{:vector}}, Tuple{T}} where T
│ Singular.R :: Union{Tuple{AbstractAlgebra.Generic.MPoly{T}}, Tuple{T}} where T<:AbstractAlgebra.RingElem
│ Singular.R :: Union{Tuple{spoly{Singular.n_unknown{T}}}, Tuple{T}} where T<:AbstractAlgebra.RingElem
I don't see why those have docstrings. I hope they proved very useful for
whoever documented them so carefully.
│ Nemo.lift :: Tuple{smodule,smodule}
Docstring is missing a capital letter on the first sentence and complete
sentences would be better. But otherwise agree.
│ AbstractAlgebra.Generic.preimage :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SAlgHom},sideal}
│ AbstractAlgebra.Generic.preimage :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SIdAlgHom},sideal}
Technically it is documented in AbstractAlgebra, but it makes sense to
document it again here. So agree, though this may mean adding a page to the
docs.
│ Singular.isquotient_ring :: Tuple{PolyRing}
Agreed, though this is hard to implement in general and is very Singular
specific, so I'm not sure how much value there is in having such a function
in an official interface.
│ Singular.slimgb :: Tuple{sideal}
│ Singular.slimgb :: Tuple{smodule}
Agreed, though I am not sure if these are available by default or were
only added for someone who wanted to use slimgb who happened to have it
installed. This should be checked (I am probably wrong).
│ Singular.substitute_variable :: Tuple{spoly,Int64,spoly}
We already provide this via AbstractAlgebra via the generic
substitution/evaluation hardware. This was added by someone who wanted to
use the Singular specific version of it, but didn't want to implement the
AbstractAlgebra interface to use this function instead of the generic
version provided by AA.
So at the very least, some work is required to fix this one, whereupon the
function should essentially disappear and be replaced by an implementation
of the proper AA version (to replace the generic one currently provided for
free, which may be faster anyway, who knows).
|
|
Some of this is also related to the question as to what Singular.jl
actually is meant to be:
- a low level interface to singular: then e.g. lift_std should be
called just that as every singular person will understand (and
similar names/ conventions, including the singular poly eval, permute
var...)
- closely aligned to Oscar: lift_std as a name makes no sense,
std_with_transform or similar would be better - which no singular
person will ever find...
What about a misc sectino in the "manual" where documenter puts all the
forgotten strings?
I'd be a shame to remove doc strings just to keep documenter happy.
Docstrings are useful even if they are not in the documentation....
I feel that I do not care too much about the unhappyness of
documenter....
…On Sun, Jan 10, 2021 at 09:16:07PM -0800, wbhart wrote:
On Mon, 21 Dec 2020 at 11:37, Max Horn ***@***.***> wrote:
> Running documenter on the manual currently produces a bunch of warnings
> about docstrings that are not included in the manual. We should probably
> fix those.
>
> ┌ Warning: 18 docstrings not included in the manual:
>
> │
>
> │ Singular.permute_variables :: Tuple{spoly,Array{Int64,1},PolyRing}
>
> This was added because someone wanted to use the corresponding Singular
function. We provide generic functionality for this via AbstractAlgebra,
under for example evaluation. It would make sense to implement such a
function in AbstractAlgebra generically though. But I'm uncertain if we
want to implement the interface like this. So this is very definitely
provisional from memory.
It was never officially documented for this reason.
>
>
> │ Singular.modulo :: Tuple{smodule,smodule}
>
> This looks like it could be related to functionality given a different
name in AbstractAlgebra, which might explain why it doesn't appear in the
docs. It's not precisely what we call a quotient module there, but it is
the equivalent for more general modules if I understand correctly.
>
>
> │ Singular.Singular
>
>
I have no idea why this has a docstring.
>
>
> │ AbstractAlgebra.Generic.kernel :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SIdAlgHom}}
>
> │ AbstractAlgebra.Generic.kernel :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SAlgHom}}
>
> Agreed, this should appear in the manual I think, though it is technically
already documented in AbstractAlgebra.
>
>
> │ Singular.lift_std :: Tuple{smodule}
>
> We are using names like blah_with_transform for functions of this kind in
our other packages. I suggest renaming it before including in docs.
>
>
> │ Singular.eliminate :: Tuple{smodule,Vararg{spoly,N} where N}
>
>
Agreed it should be included.
>
>
> │ Singular.R :: Union{Tuple{CxxWrap.CxxWrapCore.CxxPtr{Singular.libSingular.ideal},Val{:module}}, Tuple{T}} where T
>
> │ Singular.R :: Union{Tuple{CxxWrap.CxxWrapCore.CxxPtr{Singular.libSingular.poly},Val{:vector}}, Tuple{T}} where T
>
> │ Singular.R :: Union{Tuple{AbstractAlgebra.Generic.MPoly{T}}, Tuple{T}} where T<:AbstractAlgebra.RingElem
>
> │ Singular.R :: Union{Tuple{spoly{Singular.n_unknown{T}}}, Tuple{T}} where T<:AbstractAlgebra.RingElem
>
>
I don't see why those have docstrings. I hope they proved very useful for
whoever documented them so carefully.
>
>
> │ Nemo.lift :: Tuple{smodule,smodule}
>
>
Docstring is missing a capital letter on the first sentence and complete
sentences would be better. But otherwise agree.
>
>
> │ AbstractAlgebra.Generic.preimage :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SAlgHom},sideal}
>
> │ AbstractAlgebra.Generic.preimage :: Tuple{AbstractAlgebra.Map{PolyRing,PolyRing,Singular.AbstractAlgebraHomomorphism,Singular.SIdAlgHom},sideal}
>
> Technically it is documented in AbstractAlgebra, but it makes sense to
document it again here. So agree, though this may mean adding a page to the
docs.
>
>
> │ Singular.isquotient_ring :: Tuple{PolyRing}
>
> Agreed, though this is hard to implement in general and is very Singular
specific, so I'm not sure how much value there is in having such a function
in an official interface.
>
>
> │ Singular.slimgb :: Tuple{sideal}
>
> │ Singular.slimgb :: Tuple{smodule}
>
> Agreed, though I am not sure if these are available by default or were
only added for someone who wanted to use slimgb who happened to have it
installed. This should be checked (I am probably wrong).
>
>
> │ Singular.substitute_variable :: Tuple{spoly,Int64,spoly}
>
> We already provide this via AbstractAlgebra via the generic
substitution/evaluation hardware. This was added by someone who wanted to
use the Singular specific version of it, but didn't want to implement the
AbstractAlgebra interface to use this function instead of the generic
version provided by AA.
So at the very least, some work is required to fix this one, whereupon the
function should essentially disappear and be replaced by an implementation
of the proper AA version (to replace the generic one currently provided for
free, which may be faster anyway, who knows).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#317 (comment)
|
|
There also seems to be some duplication between this issue and #145 which I opened ages ago. |
We can instruct documenter to not complain via the |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Running documenter on the manual currently produces a bunch of warnings about docstrings that are not included in the manual. We should probably fix those.
The text was updated successfully, but these errors were encountered: