Restore compatibility with the matgrp package#485
Merged
Conversation
... via a crude hack. Thing is, the arguments for BindRecogMethod, as well as the inputs and outputs of recognition methods have changed. Luckily the method in matgrp does nothing very useful, so we can replicate an updated version here.
| arg[Length(arg)] := | ||
| function(ri) | ||
| if IsBound(ri!.ring) and not IsBound(ri!.field) then | ||
| Error("hereIAm"); |
Collaborator
There was a problem hiding this comment.
What does this error mean? I see that it has been copied from matgrp but I'm not sure what it means.
Member
Author
There was a problem hiding this comment.
I think it means that this is a debug left-over that should just be removed from matgrp ;-)
Perhaps @hulpke was working on support for RecognizeGroup (see also #166), and this was meant to ensure nothing of that sort crept into the wrong places. But this is just speculation.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #485 +/- ##
==========================================
- Coverage 88.17% 88.13% -0.05%
==========================================
Files 45 45
Lines 19010 19023 +13
==========================================
+ Hits 16763 16765 +2
- Misses 2247 2258 +11
🚀 New features to boost your workflow:
|
SoongNoonien
approved these changes
May 19, 2026
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
2 participants
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.
... via a crude hack.
Thing is, the arguments for BindRecogMethod, as well as the inputs and outputs of recognition methods have changed.
Luckily the method in matgrp does nothing very useful, so we can replicate an updated version here.
See also hulpke/matgrp#19 which addresses this on the matgrp side by simply removing the offending code there (it doesn't do anything useful as far as I can tell). But since we need to be compatible with the current release of matgrp, too, I am making this hackish workaround here (I've also thought about doing a "proper" adapter that is more general, but I don't know of any other external package using these APIs, so I opted to keep it simple.