Rework trait system #9
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trait changes
There is now only one
Partition
trait, which exposes apartition
function and two types:coupe/src/lib.rs
Lines 46 to 66 in 5521e9c
Algorithms' input is represented by the generic type
M
(e.g.&'a [Point2D]
, orW: IntoIterator
or even()
for the random partitioning algorithm).The function now can now mutate algorithm state. I don't know if it's something we want or not, but
Random
was using aRefCell
for the prng state.The
Error
type will be used instead of panics. I don't have modified all algorithms yet, and would rather do that in another PR.Example usage
Implementation of VNBest, which is currently the only algorithm to return diagnostic data (metadata, see #34):
coupe/src/algorithms/vn/best.rs
Lines 151 to 170 in 5521e9c
Example usage of RCB:
Limitations
Some implementations cannot be as generic as we'd want, for example k-means:
coupe/src/algorithms/k_means.rs
Line 824 in 5521e9c
At first, I wanted k-means to implement
Partition
for anyP: AsRef<[PointND<D>]>
andW: AsRef<[f64]>
(so that it can also be used with a&Vec<_>
), but rust doesn't accept those kind of "loose" constraints on const-generic parameters.I suppose this will do for now. Surprisingly, RCB, which has looser constraints, is fine:
coupe/src/algorithms/recursive_bisection.rs
Lines 593 to 602 in 5521e9c
Misc changes
All algorithm structs have been moved in their respective module, to declutter
lib.rs
.Constructors/Builder patterns have been removed since they didn't add much (and a
fn new
with 5 parameters is not that readable). It's simpler to have aDefault
impl and let the user doAlgo { specific_param, ..Algo::default() }
.