enabled quantile regression in gbm #2603
Conversation
NEWS.md
Outdated
| @@ -11,6 +11,7 @@ | |||
| See `?regr.randomForest` for more details. | |||
| `regr.ranger` relies on the functions provided by the package ("jackknife" and "infjackknife" (default)) | |||
| (@jakob-r, #1784) | |||
| - `regr.gbm` now supports `quantile distribution` | |||
There was a problem hiding this comment.
Also add your username and the PR number please.
|
Thanks. Only one comment left from my side. Please take a look why the test is failing and tell us if you need help. Looks almost ready to merge 👍 |
|
Seems to be ok (missing |
R/RLearner_regr_gbm.R
Outdated
| gbm::gbm(f, data = getTaskData(.task, .subset), ...) | ||
|
|
||
| params = list(...) | ||
| if("alpha" %in% names(params)) { |
There was a problem hiding this comment.
Space after if and I would check whether params$alpha is NULL for consistency with how it's set below.
R/RLearner_regr_gbm.R
Outdated
| gbm::gbm(f, data = getTaskData(.task, .subset), weights = .weights, ...) | ||
| alpha = 0.5 | ||
| } | ||
| if(params$distribution %in% "quantile") { |
There was a problem hiding this comment.
Space after if and should that check be simply ==?
|
For style, you can simply apply the mlr style guide. (in case you haven't already) |
|
@larskotthoff If you approve, feel free to merge. We can run the style fixer afterwards. Applying it on a forked branch is not easy. |
|
Merging. |
* enabled quantile regression in gbm * add requires quantile for alpha * style guide * mutualize * Rm quantile note * Update NEWS * gbm : add quantile test * add name + PR * fix typo using weights (and so error on tests...) * space after if + simplify condition
We are always happy to receive pull requests.
Please make sure you have read our coding guidelines:
https://github.com/mlr-org/mlr/wiki/mlr-Coding-Guidelines
This especially means that you have understood:
Also it's helpful to get into direct contact with the suggested reviewers to get help, getting your PR merged.
You might want to join our slack at:
https://mlr-org.slack.com