-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: allow creating a Box-Cox or Yeo-Johnson transform with either lambda or data #212
Conversation
…ambda or data This makes the Transform::box_cox and Transform::yeo_johnson constructors more flexible; users can either pass lambda directly or pass some data which will be used to find the optimal lambda.
WalkthroughThe pull request introduces significant enhancements to power transformation methods in the Augurs Forecaster library. The changes focus on improving the flexibility and error handling of Box-Cox and Yeo-Johnson transformations by adding new traits Changes
Sequence DiagramsequenceDiagram
participant User
participant Transform
participant Lambda Trait
participant Optimizer
User->>Transform: Call box_cox/yeo_johnson
Transform->>Lambda Trait: Convert input to lambda
Lambda Trait-->>Optimizer: Compute optimal lambda if slice provided
Optimizer-->>Lambda Trait: Return lambda value
Lambda Trait-->>Transform: Return lambda
Transform->>Transform: Apply transformation
Transform-->>User: Return transformed data or error
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/augurs-forecaster/src/transforms.rs (2)
170-170
: Potentially wrap conversion errors with more specific context.
You are returning a generic error from trait conversion, which is perfectly valid. However, consider wrapping it with a context string like “Failed to convert lambda from user input” to aid troubleshooting.Example diff for adding context:
-let lambda = lambda.into_box_cox_lambda()?; +let lambda = lambda.into_box_cox_lambda().map_err(|e| Error::msg(format!("Box-Cox lambda conversion failed: {}", e)))?;
196-197
: Mirror the same context for Yeo-Johnson conversion errors.
Similar to the Box-Cox transformation, adding context to the error handling here would give users improved clarity on why the conversion failed.Example diff:
-let lambda = lambda.into_yeo_johnson_lambda()?; +let lambda = lambda.into_yeo_johnson_lambda().map_err(|e| Error::msg(format!("Yeo-Johnson lambda conversion failed: {}", e)))?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/augurs-forecaster/src/transforms.rs
(3 hunks)crates/augurs-forecaster/src/transforms/power.rs
(1 hunks)
🔇 Additional comments (4)
crates/augurs-forecaster/src/transforms.rs (3)
26-27
: Imports for new transformation traits appear consistent.
No obvious issues are found here. Importing the newly added traits and functions follows a logical approach to keep the code modular and maintainable.
159-169
: Documentation clarifies the new flexibility for Box-Cox transformations.
The block comment and parameter details do an excellent job explaining the acceptance of either a f64 or a &[f64]. This clarity is beneficial for library users.
185-195
: Accurate explanation of Yeo-Johnson’s flexibility.
You’ve documented how negative and zero values are handled. This is essential for users. The method signature is consistent with the Box-Cox approach, which keeps the API uniform.
crates/augurs-forecaster/src/transforms/power.rs (1)
355-394
: New traits elegantly abstract lambda determination.
- Implementation for f64 allows a direct pass-through of the parameter.
- Implementation for &[f64] leverages MLE to compute the optimal parameter, which is consistent with the broader logic in this PR.
Overall, these traits reduce code duplication and provide a clean interface.
This makes the Transform::box_cox and Transform::yeo_johnson constructors more flexible; users can either pass lambda directly or pass some data which will be used to find the optimal lambda.
Summary by CodeRabbit
New Features
Bug Fixes