Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6353 +/- ##
=======================================
Coverage 78.63% 78.63%
=======================================
Files 684 684
Lines 292689 292688 -1
Branches 8686 8660 -26
=======================================
+ Hits 230153 230154 +1
Misses 60726 60726
+ Partials 1810 1808 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ThomasBreuer
left a comment
There was a problem hiding this comment.
The old code uses IdGroup only if the group in question has order at most 1000. The proposed code uses it also for larger groups, for example for groups of order p^4, where p is any prime. Is this intended?
| else | ||
| l:=[idx,fail]; | ||
| fi; | ||
| f:=Image(hom,gp); |
There was a problem hiding this comment.
This is irritating:
We have hom, and here we create its image, but a few lines above we create this group as gp/nor on the fly, just in order to call IdGroup.
There was a problem hiding this comment.
Yeah that's weird. I've rearranged the code now to avoid this.
... instead of hardcoding assumptions about the availability of IdGroup.
|
The limit to 1000 seemed arbitrary, perhaps an attempt approximate The code was added by @hulpke on 2013-01-25, in a commit with commit message |
... instead of hardcoding assumptions about the availability of
IdGroup.