-
Notifications
You must be signed in to change notification settings - Fork 77
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
New functions for checking multiple sets #1925
base: main
Are you sure you want to change the base?
Conversation
Functionality doesn't have tests. Where do the tests go for the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1925 +/- ##
=======================================
Coverage 49% 49%
- Complexity 6161 6162 +1
=======================================
Files 661 661
Lines 58698 58701 +3
Branches 8547 8548 +1
=======================================
+ Hits 28958 28961 +3
- Misses 27549 27552 +3
+ Partials 2191 2188 -3 ☔ View full report in Codecov by Sentry. |
Very nice additions and necessary to the library. Thanks @linuswagner |
Hi @linuswagner, thanks for the nice addition. Could you at at least a few tests to the lang::rascal::tests::library::Set module? |
@jurgenvinju I can't find the explanation of
it does not actually match pairs, but actually keeps |
Perfect, that's what I was looking for. Thanks! I've decided to make the operations stricter and throw exceptions when the input has not at least 2 elements. Is Rascal generally "better safe than sorry" or "you should better know what you're doing"? |
Cool 👍🏼
I think better safe than sorry. We don't want someone to base their conclusion on something where rascal was just sneakily hiding an error the user made. So if there is no default correct behavior for an input with 1 element, then yeah, throw an exception. |
public bool isPairwiseDisjoint(wholeInput:list[set[&T]] sets) { | ||
int sizeSets = size(sets); | ||
if (sizeSets == 0 || sizeSets == 1) { | ||
throw IllegalArgument(wholeInput, "Only two or more sets can be pairwise disjoint."); |
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.
sets
== wholeInput
, no need to give it two names right?
|
||
test bool testIsPairwiseDisjointIdenticalElements() {return isPairwiseDisjoint([{1}, {1}]) == false;} | ||
test bool testIsPairwiseDisjointNoOverlap() {return isPairwiseDisjoint([{1,2},{3,4},{5,6}]) == true;} | ||
test bool testIsPairwiseDisjointOverlap() {return isPairwiseDisjoint([{1,2}, {-4,5}, {1,6,7}]) == false;} |
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.
if possible, think of an way to write an random tests. something like:
test bool testRandomPairwiseDisjoint(set[value] a, set[value] b) = (a & b == {}) == isPairwiseDisjoint([a,b]);
or even better if you could do it without using the intersection operator.
Looking at this to merge the contributions. Currently wrestling with merge conflicts. |
Add two functions to
Set
library:intersection
to get the intersection of multiple setsisDisjoined
to check if multiple sets are pairwise disjoined