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

Adding an example for PRump. #2192

Merged
merged 1 commit into from
Feb 19, 2018

Conversation

grouptheoryenthusiast
Copy link
Contributor

Addressing issue #1832.
Added a simple example to the description of the PRump function to demonstrate how to call it.

@olexandr-konovalov
Copy link
Member

@grouptheoryenthusiast thank you! The test of this PR failed in https://travis-ci.org/gap-system/gap/jobs/343088351 because the example is likely sensitive to some randomised algorithms or the set of packages loaded:

########> Diff in /home/travis/build/gap-system/gap/doc/ref/../../lib/grp.gd:
3211 (/home/travis/build/gap-system/gap/tst/testmanuals/chapter39.tst:375)
# Input is:
PRump(g,2);
# Expected output:
Group([ (), (2,3,4), (1,2,3), (1,3)(2,4) ])
# But found:
Group([ (), (1,3,2), (1,3)(2,4), (1,4,3) ])
########
chapter39.tst

A simple way to resolve this is to update example to use the output below. But then in the future this will likely require amending again. I think it may be better to make an example which demonstrates some property of the output, e.g. that it is equal to G' G^p.

@markuspf
Copy link
Member

Could you squash the two commits into one? (Let me know if you need help with the git magic)

Following the feedback from AK, the example
has been updated to demonstrate a property
of the function. In particular
we explicitly show that
PRump(g,p) = G'G^p .
@grouptheoryenthusiast
Copy link
Contributor Author

Hi Markus,
I have squashed the two commit messages into one using rebase, and then I pushed the branch using: git push origin +fix/master/prump .
I am hoping that this is correct, I am still quite new to the finer details of git.
Please let me know if there is anything else I should do here.

@markuspf markuspf merged commit 16b82ab into gap-system:master Feb 19, 2018
@markuspf
Copy link
Member

All fine. Thanks!

@grouptheoryenthusiast grouptheoryenthusiast deleted the fix/master/prump branch February 19, 2018 15:18
@fingolfin fingolfin added the topic: documentation Issues and PRs related to documentation label Mar 28, 2018
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes topic: documentation Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants