Do not require the SmallGrp package - #34
Merged
Merged
Conversation
The `ViewObj' and `Display' methods for rcwa groups mention the
isomorphism type of the group if `IdGroup' happens to be known. Both the
attribute `IdGroup' and its tester `HasIdGroup' are provided by the
SmallGrp package though, so without it viewing or displaying an rcwa
group failed with
Error, Variable: 'HasIdGroup' must have an assigned value
Only SmallGrp can set that attribute, hence it is certainly not set when
the package is absent. Add the helpers `RCWA_HasIdGroup' and
`RCWA_IdGroup' which take that into account, and use them in the two
places in question; the output is unchanged when SmallGrp is available,
and falls back to printing the order otherwise.
See gap-system/gap#2434
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four tests in tst/integral.tst require SmallGrp: two call `IdGroup' directly (and check that the isomorphism type then shows up in the `ViewObj' output), and two use `StructureDescription', which produces prettier names when it can look up `NAMES_OF_SMALL_GROUPS' via `IdGroup'. Guard these using the `#@if' syntax of the test file parser. For the `IdGroup' case add an `#@else' branch, so that the fallback of printing the order rather than the isomorphism type is covered as well. See gap-system/gap#2434 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #34 +/- ##
=======================================
Coverage 99.58% 99.58%
=======================================
Files 47 47
Lines 366226 366230 +4
=======================================
+ Hits 364707 364711 +4
Misses 1519 1519
🚀 New features to boost your workflow:
|
Member
Author
|
@Stefan-Kohl are you OK with these changes, and putting them into a release? happy to answer questions |
Member
|
@fingolfin Thanks - the changes are fine with me. I have made a release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Viewing or displaying an rcwa group fails when the SmallGrp package is not loaded:
The
ViewObjandDisplaymethods inlib/rcwagrp.gimention the isomorphism type of a group ifIdGrouphappens to be known, but both that attribute and its testerHasIdGroupare provided by SmallGrp.Since only SmallGrp can set the attribute, it is certainly not set when the package is absent, so treating it as unset in that case is safe. This adds two helpers,
RCWA_HasIdGroupandRCWA_IdGroup, and uses them in the two places concerned. Output is unchanged when SmallGrp is available, and falls back to printing the order otherwise.Note the helpers deliberately go through
ValueGlobal: writingHasIdGroup(G)textually — even behind a workingIsBoundguard — still makes GAP emitSyntax warning: Unbound global variablewhile reading the file. With this change RCWA loads without any such warnings.Four tests in
tst/integral.tstalso needed SmallGrp: two callIdGroupdirectly, and two useStructureDescription, which yields prettier names when it can consultNAMES_OF_SMALL_GROUPS. These are guarded with the#@ifsyntax of the test file parser. For theIdGroupcase there is an#@elsebranch, so the new fallback output is covered too.Testing
gap --bare -c 'LoadPackage("GAPDoc");'+LoadPackage("rcwa" : OnlyNeeded)+TestPackage("rcwa")— 0 failures in 14 files, and no syntax warnings. Previously 58 differences.I have left
PackageInfo.gand the release date inCHANGES.mdalone, so this needs a version bump and release on your side.Context
This comes out of gap-system/gap#2434, which aims to make GAP and its packages work when PrimGrp/SmallGrp/TransGrp are not installed. RCWA showed up in a PackageDistro CI experiment running package test suites under
gap --bare.🤖 Generated with Claude Code