-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Split AsTargets
into AsSingleTargets
and AsMultiTargets
#203
Changes from 6 commits
f955154
365a116
b7a6e67
86b40cd
ad148cb
d28484c
d19f45f
f609700
c7dab14
d396611
01678a8
a06998d
b2c6fba
5cd01d7
47a1bb5
0e0afc7
ceeb298
bb75942
e1c6ab0
ad33a4a
e2b2f3d
f42b2bf
7ead65c
b43d410
6d69d4a
feb3c75
0ebc59e
be64d10
ed4cfe6
53ebb9c
7c6b9cf
8e80ec6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,8 +198,8 @@ pub trait Records: Sized { | |
fn nfeatures(&self) -> usize; | ||
} | ||
|
||
/// Return a reference to single or multiple target variables | ||
pub trait AsTargets { | ||
/// Return a reference to single target variable | ||
pub trait AsSingleTargets { | ||
type Elem; | ||
|
||
/// Returns a view on targets as two-dimensional array | ||
|
@@ -222,6 +222,17 @@ pub trait AsTargets { | |
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This trait should only have the method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
} | ||
|
||
/// Return a reference to a multi-target variable | ||
pub trait AsMultiTargets { | ||
type Elem; | ||
|
||
/// Convert to a multi-target | ||
fn as_multi_targets(&self) -> ArrayView2<Self::Elem>; | ||
|
||
/// Returns the number of targets | ||
fn ntargets(&self) -> usize; | ||
} | ||
|
||
/// Helper trait to construct counted labels | ||
/// | ||
/// This is implemented for objects which can act as targets and created from a target matrix. For | ||
|
@@ -236,7 +247,7 @@ pub trait FromTargetArray<'a, F> { | |
fn new_targets_view(targets: ArrayView2<'a, F>) -> Self::View; | ||
} | ||
|
||
pub trait AsTargetsMut { | ||
pub trait AsSingleTargetsMut { | ||
type Elem; | ||
|
||
/// Returns a mutable view on targets as two-dimensional array | ||
|
@@ -254,6 +265,16 @@ pub trait AsTargetsMut { | |
} | ||
} | ||
|
||
pub trait AsMultiTargetsMut { | ||
type Elem; | ||
|
||
/// Convert to a multi-target | ||
fn as_multi_targets_mut(&mut self) -> Result<ArrayViewMut2<Self::Elem>>; | ||
|
||
/// Returns the number of targets | ||
fn ntargets(&self) -> usize; | ||
} | ||
|
||
/// Convert to probability matrix | ||
/// | ||
/// Some algorithms are working with probabilities. Targets which allow an implicit conversion into | ||
|
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.
@YuhanLiin The implementation for
T: AsMultiTargets<Elem = L>
would be very similar. What do you recommend to avoid duplicating very similar code?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.
Use
AsMultiTargets
instead ofAsSingleTarget
as the bound for these dataset impls.AsMultiTarget
is more general thanAsSingleTarget
, and even without a blanket impl, types that implementAsSingleTarget
probably also implementAsMultiTarget
.