-
Notifications
You must be signed in to change notification settings - Fork 18
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
add LinearRegression class #134
Conversation
Codecov Report
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 78.97% 79.43% +0.45%
==========================================
Files 29 29
Lines 1346 1405 +59
==========================================
+ Hits 1063 1116 +53
- Misses 283 289 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This is basically an xarray wrapper for |
@yquilcaille If you are interested you can have a look here. You can concentrate on the |
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.
Looks sick, such a nice and elegant solution to how we store parameters, make the fitting and prediction stay close to each other etc. Not sure if there's changelog etc. to be done but I think this is a great pattern for us to follow
|
||
def LinearRegression_fit_wrapper(*args, **kwargs): | ||
# wrapper for LinearRegression().fit() because it has no return value - should it? |
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 um and ah about this. I think it's probably a good idea to just get people used to the pattern of calling fit
and then getting the params
or whatever in the next line. That's what's used in sklearn and following that is probably a good idea. We could add a function like this to the codebase of course, it's so simple that it probably doesn't hurt...
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.
Yes, I recently heard somewhere that a class method should either return something or change state but not both. So I will keep it as is.
Thanks for the feedback! Will add a changelog and merge. I am not yet sure if the AR process will work |
isort . && black . && flake8
CHANGELOG.rst
This adds a
LinearRegression
class. To fit a linear regression you would call:This sets
lr.params
- i.e. the estimated intercept and slope for tas.The next examples for one gridpoint ("cell") actually work - you can manually set
lt.params
and then calllr.predict
andlr.residuals
to get the predicted values and the residuals: