-
Notifications
You must be signed in to change notification settings - Fork 5
[ENH] Add CovariateRegressor class #27
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
Conversation
|
I believe the test failure is unrelated. It might be an update to pytorch API that I need to investigate, before we can have your code run cleanly. |
|
@arokem I believe that it’s because |
|
@chiuhoward : do you want to make a separate PR just to fix that failure first, and then we can move on with this one? BTW: I gave it a first read through and the code in this PR looks great. I will give it a thorough review once we resolve the test failures, if that's OK with you. |
|
OK - now that #29 is merged, you can either rebase this one on top of main, or merge main into this branch and then push again, so we can see that all tests are passing with that fix. |
- Implements CovariateRegressor for regressing covariates from features - Includes find_subset_indices utility for hash and precise matching - Adds IdentityTransformer helper class - Tests cover edge cases and error handling - Supports cross-validation, missing value imputation, and flexible pipelines - All tests pass with full coverage of core functionality
7d8ad3b to
b629b66
Compare
arokem
left a comment
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 have not read through the tests yet, but here are a few comments for now.
|
Thanks for your work on this @chiuhoward! |
With some assistance from @richford