Skip to content

Replaced FrobHelp with AutoDoc#11

Merged
AliaBonnet merged 17 commits into
mainfrom
fix/issues4-9
Nov 30, 2025
Merged

Replaced FrobHelp with AutoDoc#11
AliaBonnet merged 17 commits into
mainfrom
fix/issues4-9

Conversation

@AliaBonnet
Copy link
Copy Markdown
Collaborator

FrobHelp function has been replaced by AutoDoc comments.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 30, 2025

Codecov Report

❌ Patch coverage is 97.87234% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.81%. Comparing base (cbc1e6f) to head (1265af7).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
gap/nofoma.gi 94.44% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #11       +/-   ##
===========================================
+ Coverage   18.35%   88.81%   +70.45%     
===========================================
  Files           6        6               
  Lines         877      617      -260     
===========================================
+ Hits          161      548      +387     
+ Misses        716       69      -647     

☔ 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.

Comment thread doc/nofoma.xml Outdated
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8"?>

<!-- This is an automatically generated file. -->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should not be added to the repository, no title.xml. Please remove both files from the PR (I've now updated .gitignore on main to exclude these files)

Comment thread gap/nofoma.gd Outdated
#! @Description
#! 'GcdCoprimeSplit' computes a divisor <M>a_1</M> of the polynomial <M>a</M> and a
#! divisor <M>b_1</M> of the polynomial <M>b</M> such that <M>a_1</M> and <M>b_1</M> are coprime
#! and the lcm of <M>a</M>, <M>b</M> is <M>a_1</M>*<M>b_1</M>. This is based on Lemma 5 in <Cite Key = "Bon14"/>.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no bibliography file in this PR and hence the Cite commands don't work and even produce an error.

Also, normally XML attributes don't use spaces around the =:

Suggested change
#! and the lcm of <M>a</M>, <M>b</M> is <M>a_1</M>*<M>b_1</M>. This is based on Lemma 5 in <Cite Key = "Bon14"/>.
#! and the lcm of <M>a</M>, <M>b</M> is <M>a_1</M>*<M>b_1</M>. This is based on Lemma 5 in <Cite Key="Bon14"/>.

Comment thread gap/nofoma.gd Outdated
Comment on lines +100 to +102
#! [ x^5-4*x^4+5*x^3-2*x^2, # the (monic) gcd
#! x^4-6*x^3+12*x^2-10*x+3, # a1
#! x^7-12*x^6+56*x^5-128*x^4+144*x^3-64*x^2 ] # b1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments in the output make it impossible to use this example for automated testing, and also make it harder to automatically update the test output should it become necessary (e.g. because some output orders changed etc.)

Suggested change
#! [ x^5-4*x^4+5*x^3-2*x^2, # the (monic) gcd
#! x^4-6*x^3+12*x^2-10*x+3, # a1
#! x^7-12*x^6+56*x^5-128*x^4+144*x^3-64*x^2 ] # b1
#! [ x^5-4*x^4+5*x^3-2*x^2, # the (monic) gcd
#! x^4-6*x^3+12*x^2-10*x+3, # a1
#! x^7-12*x^6+56*x^5-128*x^4+144*x^3-64*x^2 ] # b1

Comment thread gap/nofoma.gd Outdated

#! @Arguments a,b
#! @Description
#! 'GcdCoprimeSplit' computes a divisor <M>a_1</M> of the polynomial <M>a</M> and a
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to repeat the function name here, as AutoDoc automatically inserts it in the generated documentation. In fact I'd consider it an anti-pattern: if we rename a function, there is a chance we forget to adjust the documentation, and then the two diverge needlessly.

Suggested change
#! 'GcdCoprimeSplit' computes a divisor <M>a_1</M> of the polynomial <M>a</M> and a
#! Computes a divisor <M>a_1</M> of the polynomial <M>a</M> and a

Comment thread gap/nofoma.gd Outdated
#! @Arguments A,pol,v
#! @Description
#! 'PolynomialToMatVec' returns the row vector obtained by multiplying
#! the row vector <M>v</M> with the matrix <M>pol</M>(<M>A</M>), where <M>pol</M> is the list
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since A, pol, v are arguments, the correct XML elements to wrap them in is <A>. So e.g.

Suggested change
#! the row vector <M>v</M> with the matrix <M>pol</M>(<M>A</M>), where <M>pol</M> is the list
#! the row vector <A>v</A> with the matrix <M><A>pol</A>(<A>A</A>)</M>, where <A>pol</A> is the list

Comment thread gap/nofoma.gd Outdated
Comment on lines +113 to +116
#! gap> A:=([ [ 0, 1, 0, 1 ],
#! gap> [ 0, 0, 0, 0 ],
#! gap> [ 0, 1, 0, 1 ],
#! gap> [ 1, 1, 1, 1 ] ]);;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parenthesis around the matrix are unnecessary and atypical, better don't do it.

Suggested change
#! gap> A:=([ [ 0, 1, 0, 1 ],
#! gap> [ 0, 0, 0, 0 ],
#! gap> [ 0, 1, 0, 1 ],
#! gap> [ 1, 1, 1, 1 ] ]);;
#! gap> A:= [ [ 0, 1, 0, 1 ],
#! gap> [ 0, 0, 0, 0 ],
#! gap> [ 0, 1, 0, 1 ],
#! gap> [ 1, 1, 1, 1 ] ];;

Comment thread gap/nofoma.gd Outdated
#! 'CyclicChainMat' repeatedly applies 'SpinMatVector1' (relative version
#! of 'SpinMatVector') to compute a chain of cyclic subspaces. The output
#! is a triple [B,C,svec] where C is such that C*<M>A</M>*C^-1 has a block
#! triangular shape with companian matrices along the diagonal), B is the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo -- I suggest to run a spell checker over this document

Suggested change
#! triangular shape with companian matrices along the diagonal), B is the
#! triangular shape with companion matrices along the diagonal), B is the

Comment thread gap/nofoma.gd Outdated
#! @Description
#! 'CyclicChainMat' repeatedly applies 'SpinMatVector1' (relative version
#! of 'SpinMatVector') to compute a chain of cyclic subspaces. The output
#! is a triple [B,C,svec] where C is such that C*<M>A</M>*C^-1 has a block
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#! is a triple [B,C,svec] where C is such that C*<M>A</M>*C^-1 has a block
#! is a triple <C>[B,C,svec]</C> where <M>C</M> is such that <M>C <A>A</A> C^-1</M> has a block

Comment thread gap/nofoma.gd Outdated

#! @Arguments A
#! @Description
#! 'CyclicChainMat' repeatedly applies 'SpinMatVector1' (relative version
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since SpinMatVector1 is not documented I would not mention it here, but rather explain what CyclicChainMat does without referring to this unknown.

Comment thread gap/nofoma.gd Outdated
#! gap> A:=[ [ 0, 1, 0, 1 ],
#! gap> [ 0, 0, 1, 0 ],
#! gap> [ 0, 1, 0, 1 ],
#! gap> [ 1, 1, 1, 1 ] ]);;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax error -- I'll make a PR to enable extraction of examples for testing, then we can see what all fails and fix these incrementally.

Suggested change
#! gap> [ 1, 1, 1, 1 ] ]);;
#! gap> [ 1, 1, 1, 1 ] ];;

Comment thread gap/nofoma.gd Outdated
Comment on lines +441 to +443
<Bibliography Databases="../doc/nofoma.bib"/>


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong place to put this (and in fact a syntax error).

But luckily AutoDoc automatically finds the bib if it is named PKGNAME.bib, where PKGNAME is the package name as defined in PackageInfo.g -- which here amounts to nofoma.xml. So it automatically finds the file, and you can just remove this here.

Suggested change
<Bibliography Databases="../doc/nofoma.bib"/>

Comment thread tst/tests.tst
@fingolfin
Copy link
Copy Markdown
Member

This PR now does a lot more than its description suggests...

@AliaBonnet AliaBonnet merged commit 5f4d0bf into main Nov 30, 2025
9 checks passed
@AliaBonnet AliaBonnet deleted the fix/issues4-9 branch November 30, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants