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.

Sign up for GitHub

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

Jump to bottom

documentation of IsPrimitive improved #3363

Merged
merged 2 commits into from
Mar 21, 2019
Merged

documentation of IsPrimitive improved #3363

merged 2 commits into from
Mar 21, 2019

Conversation

PaulaHaehndel
Copy link
Contributor

@PaulaHaehndel PaulaHaehndel commented Mar 20, 2019 •
edited
Loading

I added some information in the documentation of IsPrimitive.
While doing so I noticed that in the explanation of "xset" (external set) in the begin of the section "extset" was used that has no appearence in the rest of the document. So I changed it.
Closes #3329

@wilfwilson
Copy link
Member

wilfwilson commented Mar 20, 2019 •
edited
Loading

I believe this resolves the remaining problems in Issue #3329. Is that right @PaulaHaehndel, or is there still stuff mentioned in that issue that should be fixed?

fingolfin
fingolfin requested changes Mar 20, 2019
View reviewed changes
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Thank you, much appreciated! I have some remarks and questions, though.

doc/ref/grpoper.xml Show resolved Hide resolved
lib/oprt.gd Outdated Show resolved Hide resolved
lib/oprt.gd Outdated Show resolved Hide resolved
lib/oprt.gd Outdated
## &nbsp;<Ref Sect="About Group Actions"/>.
## <P/>
## <E>Note:</E> This operation does not tell whether a matrix group
## preserves a direct sum of vector spaces.
Copy link
Member

Choose a reason for hiding this comment

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

I am confused by this remark. Why would it tell that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It never would, but as a naive user you could think that IsPrimitive for a matrix group actually tells you whether this group is a primitive matrix group. But this is not the case.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I did not make this connection, so this paragraph, which is meant as a clarification, to me was instead confusing :/. Perhaps it could be changed to something more like this: "This operation is not concerned with the notion of "primitive vector space", i.e., it does not..."

@PaulaHaehndel PaulaHaehndel force-pushed the doc branch 2 times, most recently from b195131 to 1eb6cfd Compare March 20, 2019 16:31
@PaulaHaehndel
Copy link
Contributor Author

@wilfwilson

I believe this resolves the remaining problems in Issue #3329. Is that right @PaulaHaehndel, or is there still stuff mentioned in that issue that should be fixed?

yes it solves the remaining problems, but it still would be nice to get the information that you tried to use Blocks for an intransitive group.

@wilfwilson
Copy link
Member

The blocks problem is now its own issue: #3364.

wilfwilson
wilfwilson approved these changes Mar 20, 2019
View reviewed changes
Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

I'm happy with this. Thanks!

@PaulaHaehndel
Copy link
Contributor Author

The blocks problem is now its own issue: #3364.

Then the issue: #3329 can be closed when this PR is merged.

@fingolfin fingolfin added the gapdays2019-spring Issues and PRs that arose at https://www.gapdays.de/gapdays2019-spring label Mar 20, 2019
@coveralls
Copy link

coveralls commented Mar 20, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 85.146% when pulling 923eebf on PaulaHaehndel:doc into e3ac220 on gap-system:master.

@fingolfin
Copy link
Member

If you put "Closes #3329" or "Fixes #3329" or "Resolves #3329" into the issue description (not the title), then merging this PR will automatically close the issue -- see https://help.github.com/en/articles/closing-issues-using-keywords

@PaulaHaehndel
Copy link
Contributor Author

If you put "Closes #3329" or "Fixes #3329" or "Resolves #3329" into the issue description (not the title), then merging this PR will automatically close the issue -- see https://help.github.com/en/articles/closing-issues-using-keywords

@fingolfin did so. Is there anything else that has to be fixed with this PR?

@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #3363 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3363      +/-   ##
==========================================
- Coverage   85.14%   85.14%   -0.01%     
==========================================
  Files         697      697              
  Lines      344047   344064      +17     
==========================================
+ Hits       292950   292962      +12     
- Misses      51097    51102       +5
Impacted Files Coverage Δ
lib/oprt.gd 95.34% <ø> (ø) ⬆️
src/system.c 73.5% <0%> (-0.32%) ⬇️
src/modules.c 76.53% <0%> (-0.23%) ⬇️
src/io.c 77.42% <0%> (-0.18%) ⬇️
src/objects.c 81.33% <0%> (-0.14%) ⬇️
src/plist.c 93.18% <0%> (-0.09%) ⬇️
lib/test.gi 80.74% <0%> (ø) ⬆️
src/trans.cc 99.8% <0%> (ø) ⬆️
lib/matint.gi 84.02% <0%> (+0.02%) ⬆️

@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: documentation Issues and PRs related to documentation labels Mar 21, 2019
@wilfwilson wilfwilson merged commit 302b158 into gap-system:master Mar 21, 2019
@wilfwilson
Copy link
Member

Thanks!

@PaulaHaehndel PaulaHaehndel deleted the doc branch March 21, 2019 14:06
@PaulaHaehndel PaulaHaehndel added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Apr 15, 2019
@PaulaHaehndel PaulaHaehndel added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Apr 24, 2019
@PaulaHaehndel PaulaHaehndel mentioned this pull request Aug 5, 2019
Merged
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fingolfin fingolfin fingolfin approved these changes

@wilfwilson wilfwilson wilfwilson approved these changes

Assignees
No one assigned
Labels
gapdays2019-spring Issues and PRs that arose at https://www.gapdays.de/gapdays2019-spring kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements 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
Milestone
GAP 4.11.0
Development

Successfully merging this pull request may close these issues.

Documentation for IsPrimitive is confusing
5 participants
@PaulaHaehndel @wilfwilson @coveralls @fingolfin @olexandr-konovalov