-
Notifications
You must be signed in to change notification settings - Fork 160
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
Random source for additive cosets, grpnice and external orbits #2272
Random source for additive cosets, grpnice and external orbits #2272
Conversation
- Changing Random for Additive Cosets to use InstallWithRandomSource. - Adding tests for Random on Additive Cosets ino random.tst . - Adding a missing comma in coll.gi.
Adding InstallWithRandomSource for grpnice. All other changes had already been implemented, and suitable tests already existed.
Thanks, this looks great. I'll just wait for the tests to cycle. Thanks for poking at this fairly horrible area. Hopefully translating other random methods is going to get easier from here! |
-Changing Random for external orbits to use install with random source. - Adding tests.
@ChrisJefferson thank you :) - it did end up being trickier than I had expected. |
Codecov Report
@@ Coverage Diff @@
## master #2272 +/- ##
==========================================
+ Coverage 70.86% 70.86% +<.01%
==========================================
Files 480 480
Lines 253246 253266 +20
==========================================
+ Hits 179455 179484 +29
+ Misses 73791 73782 -9
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This begins to address some of the points raised in issue #1098 .
In particular we: