-
Notifications
You must be signed in to change notification settings - Fork 35
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
Broken IsTrivial
for empty semigroups
#459
Comments
@fingolfin thanks for the report, the empty semigroup rears its head again! Happy with your proposed fix, also will be happy to implement this if you don't have time |
I would appreciate if you could take care of it. I am never sure which branche(s) I should target with fixes. Also, not sure where you'd like to have a test case. BTW, we noticed this because it broke a new test case in GAP master. |
The broken test can be seen on travis here: https://travis-ci.org/gap-system/gap-docker-master-testsuite/builds/342310829 (that's a travis cron build which now performs testinstall/standard/bugfix with three combinations of packages: none, default and all loaded. |
Fixing it now, and, of course, happy to do so. For the record @fingolfin I think we are using the same branch structure as GAP itself, bug fixes go into |
This is resolved by PR #460. |
Thanks @james-d-mitchell . But for the record, this is not how GAP works: We do all work on the Also, you have two (semi-)active stable branches, 2.8 and 3.0 (or at least you had -- but perhaps now 2.8 is closed), which added to my uncertainty -- i.e. did not know into which subset of 2.8, 3.0, master you want this fix. So I thought it'd save everybody time to just mention the issue and leave it to you guys to know where you want this :-). Anyway, I'll submit PRs again in the future, too, and if I select the wrong branch, I am sure you'll guys will let me know and I can then simply rebase :-). |
Without semigroups loaded:
But with semigroups loaded, I get an error due to the last line of this method provided by semigroups:
I guess a simple fix would be to add
Length(gens) > 0 and
to the condition in the return. And I assume a test case should be added, too?The text was updated successfully, but these errors were encountered: