Clarify LocalNR manual entries#47
Conversation
Improve the manual text for LocalNR by fixing grammar, making library references explicit, and explaining several return values and parameter conventions more precisely. Clarify that AdditiveGroupsOfLibraryOfLNRsOfOrder returns groups. Fill in or tighten several function descriptions in the manual source and regenerate the extracted example tests accordingly. Co-authored-by: Codex <codex@openai.com>
|
|
||
| #! @Description | ||
| #! The argument is $n$. | ||
| #! The output a list of <C>IdGroup</C> of the additive groups |
There was a problem hiding this comment.
@fingolfin but the output is a list of group IDs, not of groups themselves
There was a problem hiding this comment.
Maybe
#! The output is a list of group IDs of additive groups of local nearrings in the
#! library of this package of order $n$.
?
There was a problem hiding this comment.
No, the output is a list of groups, as the new text says, not a list of group ids. Look at the example a bit below, which takes the output of this function, then applies IdGroup to it, to obtain a list of ids.
|
|
||
| #! @Description | ||
| #! The argument is a group $G$. | ||
| #! The output is |
There was a problem hiding this comment.
This was incomplete as pointed out in issue #39
There was a problem hiding this comment.
Look good - even if this function may disappear later.
There was a problem hiding this comment.
@raemarina @IrynaRaievska I know that you intended to eliminate this function, but with Max's fix the documentation is complete. You can keep it, and then optimise the other function (IsEndoCyclicGroup) later to avoid redundant checks.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #47 +/- ##
=======================================
Coverage 96.98% 96.98%
=======================================
Files 5 5
Lines 664 664
=======================================
Hits 644 644
Misses 20 20
🚀 New features to boost your workflow:
|
|
|
||
| #! @Description | ||
| #! The argument is $n$. | ||
| #! The output a list of <C>IdGroup</C> of the additive groups |
There was a problem hiding this comment.
@fingolfin but the output is a list of group IDs, not of groups themselves
| #! The output is local nearring from <C>Library</C> without | ||
| #! check. The arguments $k$, $l$, $m$, $n$ are from IdGroup of the additive group and the multiplicative group, | ||
| #! respectively, $w$ is the position in the list. | ||
| #! The output is the $w$-th local nearring from the library of this |
There was a problem hiding this comment.
A minor nitpick, I'd not usually use w for an integer index :) What about i or j ?
There was a problem hiding this comment.
That kind of change is out of scope for this PR. The package authors called this w, and I'd rather not mess with it here, that can be done in a follow-up PR
|
|
||
| #! @Description | ||
| #! The argument is a group $G$. | ||
| #! The output is |
There was a problem hiding this comment.
Look good - even if this function may disappear later.
|
|
||
| #! @Description | ||
| #! The argument is a group $G$. | ||
| #! The output is |
There was a problem hiding this comment.
@raemarina @IrynaRaievska I know that you intended to eliminate this function, but with Max's fix the documentation is complete. You can keep it, and then optimise the other function (IsEndoCyclicGroup) later to avoid redundant checks.
|
|
||
| #! @Description | ||
| #! The argument is $n$. | ||
| #! The output a list of <C>IdGroup</C> of the additive groups |
There was a problem hiding this comment.
Maybe
#! The output is a list of group IDs of additive groups of local nearrings in the
#! library of this package of order $n$.
?
olexandr-konovalov
left a comment
olexandr-konovalov
left a comment
There was a problem hiding this comment.
This looks perfectly fine to me. @IrynaRaievska @raemarina you can do "rebase and merge" and seems like it will be ready for release? You still plan some updates in the README though?
2 participants
Improve the manual text for LocalNR by fixing grammar, making library references explicit, and explaining several return values and parameter conventions more precisely.
Clarify that AdditiveGroupsOfLibraryOfLNRsOfOrder returns groups. Fill in or tighten several function descriptions in the manual source and regenerate the extracted example tests accordingly.
Co-authored-by: Codex codex@openai.com
Resolves #26. Resolves #39.
All work in this PR done by Codex. I had a look over it and it seemed sensible, but you should check it carefully yourself.