-
Notifications
You must be signed in to change notification settings - Fork 2
Increased transform flexibility #13
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -61,6 +61,8 @@ | |
| //! - Z-Score Normalization: [`NormalizationTransform::ZScore`] | ||
| //! - Normalizes the dataset to zero mean and unit variance. | ||
| //! - [`ApplyNormalization::apply_z_score_normalization`] allows you to apply it to the Y channel of an (X, Y) dataset | ||
| use std::borrow::BorrowMut; | ||
|
|
||
| use crate::value::Value; | ||
|
|
||
| mod noise; | ||
|
|
@@ -87,24 +89,35 @@ pub trait Transform<T: Value> { | |
| } | ||
| } | ||
|
|
||
| /// Allow passing an `&impl Transform` to anything that wants `impl Transform`. | ||
| /// | ||
| /// This way if you don't need to re-use a transform, you can pass it directly, | ||
| /// but you can still pass large ones by reference where that's helpful. | ||
| impl<T: Value, R: ?Sized + Transform<T>> Transform<T> for &R { | ||
| fn apply<'a>(&self, data: impl Iterator<Item = &'a mut T>) { | ||
| <R as Transform<T>>::apply(*self, data); | ||
| } | ||
| } | ||
|
|
||
| /// Trait for transforming data. | ||
| pub trait Transformable<T: Value> { | ||
| /// Transforms the data in place. | ||
| fn transform<R: Transform<T>>(&mut self, transform: &R); | ||
| fn transform<R: Transform<T>>(&mut self, transform: R); | ||
|
|
||
| /// Returns a transformed copy of the data. | ||
| #[must_use] | ||
| fn transformed<R: Transform<T>>(&self, transform: &R) -> Self | ||
| fn transformed<R: Transform<T>>(&self, transform: R) -> Self::Owned | ||
| where | ||
| Self: Sized + Clone, | ||
| Self: ToOwned, | ||
| Self::Owned: BorrowMut<Self>, | ||
| { | ||
| let mut new_data = self.clone(); | ||
| new_data.transform(transform); | ||
| let mut new_data = self.to_owned(); | ||
|
Contributor
Author
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. annot: note specifically here that it's still making a copy of it https://doc.rust-lang.org/std/borrow/trait.ToOwned.html
Owner
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. Ok fair point - I concede that the ToOwned version is more general. However I still want to keep the &R here
Contributor
Author
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. To me the thing that enforces that is that https://docs.rs/polyfit/latest/polyfit/transforms/trait.Transform.html#tymethod.apply takes I just got sad having to do |
||
| new_data.borrow_mut().transform(transform); | ||
| new_data | ||
| } | ||
| } | ||
| impl<T: Value> Transformable<T> for Vec<(T, T)> { | ||
| fn transform<R: Transform<T>>(&mut self, transform: &R) { | ||
| impl<T: Value> Transformable<T> for [(T, T)] { | ||
| fn transform<R: Transform<T>>(&mut self, transform: R) { | ||
| transform.apply(self.iter_mut().map(|(_, y)| y)); | ||
| } | ||
| } | ||
|
|
@@ -228,3 +241,15 @@ impl SeedSource { | |
| Some(out) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_transformed_slice_gives_vec() { | ||
| let points = [(-0.5, -4.0), (1.0, 3.0), (5.0, 4.0)]; | ||
| let new_points: Vec<_> = points[1..].transformed(ScaleTransform::Quadratic(0.5)); | ||
| assert_eq!(new_points, [(1.0, 4.5), (5.0, 8.0)]); | ||
| } | ||
| } | ||
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.
Readme is a generated file (cargo rdme), please don't directly alter it - also this instance is already gone
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.
I did this with
cargo rdmebecause the PR build failed originally without it.