Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve nice gens functionality and docs #278

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ssiccha
Copy link
Collaborator

@ssiccha ssiccha commented Jun 19, 2021

So far three commits:

  • Turn CalcNiceGens(r,g) into NiceGens(r,g)
    • That is delete the function CalcNiceGens and instead install it as a method for NiceGens with two arguments. Also improves the documentation of NiceGens.
  • WIP Remove a line duplicating CalcNiceGensHomNode
    • I need to double-check whether this is correct but the tests look fine.
  • WIP Improve documentation of calcnicegens
    • This is still work in progress.

After that I suggest to rename as follows:
NiceGens -> NiceGenerators
calcnicegens -> NiceGeneratorsFunction
CalcNiceGensGeneric -> NiceGeneratorsForLeafRecogNodes
CalcNiceGensHomNode -> NiceGeneratorsForSplitRecogNodes
pregensfac -> PreImagesOfNiceGeneratorsOfImageRecogNode

Here the corresponding input for mine and @FriedrichRober's script so that I don't loose it:

NiceGens,NiceGenerators,Attr
calcnicegens,NiceGeneratorsFunction,Attr
CalcNiceGensGeneric,NiceGeneratorsForLeafRecogNodes,Func
CalcNiceGensHomNode,NiceGeneratorsForSplitRecogNodes,Func
pregensfac,PreImagesOfNiceGeneratorsOfImageRecogNode,Attr

@codecov
Copy link

codecov bot commented Jun 19, 2021

Codecov Report

Merging #278 (c64a1f6) into master (c57a2be) will decrease coverage by 0.00%.
The diff coverage is 90.90%.

❗ Current head c64a1f6 differs from pull request most recent head c125c8c. Consider uploading reports for the commit c125c8c to get more accurate results

@@            Coverage Diff             @@
##           master     #278      +/-   ##
==========================================
- Coverage   77.86%   77.86%   -0.01%     
==========================================
  Files          43       43              
  Lines       18389    18388       -1     
==========================================
- Hits        14319    14318       -1     
  Misses       4070     4070              
Impacted Files Coverage Δ
gap/base/recognition.gd 100.00% <ø> (ø)
gap/base/recognition.gi 68.79% <88.88%> (ø)
gap/generic/KnownNilpotent.gi 97.18% <100.00%> (ø)

ssiccha added 12 commits June 22, 2021 12:52
That is install it as a method for `NiceGens` with two arguments.

Also improves the documentation of `NiceGens`.
CalcNiceGensGeneric is the method for leaf nodes. So that it doesn't
make sense to set this for every node by default. Maybe it once had a
different function?
Will be handled during the renaming project.
@ssiccha
Copy link
Collaborator Author

ssiccha commented Jun 23, 2021

It looks like this will become a bigger undertaking. 😅 For now I'll add a lot of experimental code to this PR.

@ssiccha
Copy link
Collaborator Author

ssiccha commented Jul 6, 2021

When this is done, @aniemeyer would like to have a look at the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant