-
-
Notifications
You must be signed in to change notification settings - Fork 701
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
[WIP] Fix Issue 6718 - nWayUnion => nWayMerge, plus true nWayUnion #5620
Conversation
Thanks for your pull request, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. Bugzilla references
|
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.
Please mind the comments - thx!
std/algorithm/setops.d
Outdated
@@ -857,6 +857,46 @@ NWayUnion!(less, RangeOfRanges) nWayUnion | |||
} | |||
|
|||
/** | |||
Computes the intersection of multiple ranges. The input ranges are passed |
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.
s/intersection/union/
std/algorithm/setops.d
Outdated
@@ -857,6 +857,46 @@ NWayUnion!(less, RangeOfRanges) nWayUnion | |||
} | |||
|
|||
/** | |||
Computes the intersection of multiple ranges. The input ranges are passed | |||
as a range of ranges and each is assumed to be sorted by $(D | |||
less). Computation is done lazily, one intersection element at a time. |
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.
s/intersection/union/
std/algorithm/setops.d
Outdated
as a range of ranges and each is assumed to be sorted by $(D | ||
less). Computation is done lazily, one intersection element at a time. | ||
The `multiwayUnion` function is implemented by piping the result | ||
of $(LREF nWayUnion) through $(REF uniq std,algorithm,iteration). |
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.
Delete this, we don't need to make such guarantees of implementation. Just say "multiwayUnion(ror)
is functionally equivalent to multiwayMerge(ror).uniq
".
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.
Roger.
@andralex I suggest the following timeline:
Doing so we will close the bug issue after this is merged and all PRs will have focused targets. What do you say? |
Q: Why do we need the |
Hmmm... this seems to be stuck in circleci. |
Jenkins isn't even appearing, so it seems like a GitHub issue... |
This is the second part of solving the above metioned issue. Adds the function multiwayUnion which computes the true union of 2 ranges. The implementation returns the result of nWayUnion piped through uniq. This will be updated after #5619 is merged.
Next step is to add a multiwayIntersection function which computes the intersection of sets.