Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expression handling in mlr #1576

Closed
wants to merge 28 commits into from
Closed

Expression handling in mlr #1576

wants to merge 28 commits into from

Conversation

jakob-r
Copy link
Member

@jakob-r jakob-r commented Mar 6, 2017

Reopening #1126 after a reverted merge

Still open issues:

  • n should not refer to the task size but maybe to the train set size.

kerschke and others added 23 commits August 15, 2016 15:36
@kerschke @mllg
initial version of expression-learners
17fe5e0
@kerschke @mllg
allow expressions within tuning param sets
b56128d
@kerschke @mllg
updated documentation of expression-related files
62d6bdb
@kerschke @mllg
fixing naming issues
8b20d6f
@kerschke @mllg
further doc fixes
f85ad87
@kerschke @mllg
remove dict argument
a13c4eb
@kerschke @mllg
updating man-pages
eb9ec14
@kerschke @mllg
rm dict_template
eaea99e
@kerschke @mllg
better documentation of expression-related functions
7227a9c
@kerschke @mllg
removed ParamHelpers:: as it is not necessary
00d92f8
@mllg
cleanup; added test
e0b4728
@mllg
removed duplicated PH dep
43e2f4f
@kerschke
fixes as requested in PR#1126
8450107
@mllg
Merge branch 'master' into expression
d7ca6cf
@mllg
use setLearnerId in tests
3cfe85d
@mllg
fix warnings in tests and r cmd check
bce6633
@jakob-r
Merge branch 'master' into expression
d72e6fd 
# Conflicts:
#	DESCRIPTION
#	R/Learner_properties.R
#	tests/testthat/test_base_measures.R
@jakob-r
added assertion for Task
10d81d5
@jakob-r
Merge branch 'master' into expression
42c92e4
@mllg
Merge remote-tracking branch 'origin/master' into expression
7eb94f5
@mllg
Merge branch 'expression' of github.com:mlr-org/mlr into expression
5144137
@mllg
docs, mini cleanup
cb5fa80
@jakob-r
Merge branch 'master' into expression
641ec33
@jakob-r jakob-r mentioned this pull request Mar 6, 2017
Closed
jakob-r
jakob-r commented Mar 8, 2017
View reviewed changes
Copy link
Member Author

@jakob-r jakob-r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that with some wrappers we have train -> trainLearner -> train -> trainLearner calls. Are we using the right dictionary?

R/Task_operators.R Outdated
getTaskDictionary = function(task) {
#' getTaskDictionary(task, subset = NULL)
getTaskDictionary = function(task, subset = NULL) {
getSubsetSize = function(subset) if (is.logical(subset)) sum(subset) else length(subset)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a PR and we now have checkTaskSubset. This might want to be used here.

@larskotthoff
Copy link
Member

What's the status here @jakob-r ?

@jakob-r
Copy link
Member Author

jakob-r commented Jul 21, 2017

The status was that it is quite hard to make sure that p and n are the actual values for number of features and number of observations because of all the wrappers and resampling going on. Some heavy lifting would be required here. So we will do it in mlrNG?

@larskotthoff
Copy link
Member

Maybe something for the next workshop?

@larskotthoff
Copy link
Member

Looks like this isn't going to happen. Unless somebody objects, I'll close this soon.

@jakob-r
Copy link
Member Author

jakob-r commented Dec 14, 2017

I guess you are right. But please do not delete the branch as it contains pretty much code that might be valuable to deal with expressions outside of mlr (in PH they are allowed at least)

@larskotthoff
Copy link
Member

Ok, closing and leaving branch.

@pat-s pat-s deleted the expression branch October 4, 2018 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
pr-work in progress - not done prio-medium project - mlrWorkshop2017 type-enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jakob-r @larskotthoff @Coorsaa @kerschke @mllg