-
-
Notifications
You must be signed in to change notification settings - Fork 9
- Improve PipeOpFilter docu #135
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
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Thanks! I merged Michels PR before and now there are some conflicts but I guess you should be able to handle them 🙂
R/FilterMRMR.R
Outdated
#' library("mlr3pipelines") | ||
#' task = mlr3::tsk("spam") | ||
#' | ||
#' # Note: The filter.frac is selected randomly and should be tuned. |
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.
You can leave the "the" out here and in all other occasions, just use filter.frac
(formatting the argument as code would also be nice).
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.
done
R/FilterNJMIM.R
Outdated
#' as.data.table(filter) | ||
#' } | ||
#' | ||
#' if (mlr3misc::require_namespaces(c("mlr3pipelines", "MASS", "praznik"), quietly = TRUE)) { |
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.
What is exactly is {MASS} needed for in all the examples? Do we only need it in some?
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.
Ah sorry, I somehow though rpart was in MASS but it isn't.
README.md
Outdated
## Warning in `[.data.table`(as.data.table(mlr_filters), , !c("param_set", : | ||
## column(s) not removed because not found: [param_set] |
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.
Note this warning
In many cases filtering is only one step in the modeling pipeline. To | ||
select features based on filter values, one can use | ||
[`PipeOpFilter`](https://mlr3pipelines.mlr-org.com/reference/mlr_pipeops_filter.html) | ||
from [mlr3pipelines](https://github.com/mlr-org/mlr3pipelines). |
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.
One sentence per line please :) (maybe worth checking your editor settings, it seems line breaks are enforced at 80 chrs?)
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.
Note that this is the md file generated from the Rmd
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for some reason I cannot build the README.html because of a http error |
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.
Thanks Seb! 👍
No description provided.