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

Permutation #70

Merged
merged 7 commits into from
Jul 17, 2020
Merged

Permutation #70

merged 7 commits into from
Jul 17, 2020

Conversation

be-marc
Copy link
Member

@be-marc be-marc commented Apr 13, 2020 •
edited
Loading

Adds simple permutation filter

mllg
mllg requested changes Apr 13, 2020
View reviewed changes
R/FilterPermutation.R Outdated
#' @description
#' Estimate how important individual features are by contrasting prediction
#' performances. Compute the change in performance from permuting the values of
#' a feature and compare that to the predictions made on the unmodified data.
Copy link
Member

Choose a reason for hiding this comment

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

Not clearly documented on what data the learner is applied. Only on a single permuted col or on the complete task with one column permuted?

Copy link
Member Author

Choose a reason for hiding this comment

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

The latter. Updated the description.

@codecov-io
Copy link

codecov-io commented Apr 15, 2020 •
edited
Loading

Codecov Report

Merging #70 into master will decrease coverage by 0.33%.
The diff coverage is 97.22%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #70      +/-   ##
===========================================
- Coverage   100.00%   99.66%   -0.34%     
===========================================
  Files           20       21       +1     
  Lines          265      301      +36     
===========================================
+ Hits           265      300      +35     
- Misses           0        1       +1
Impacted Files Coverage Δ
R/FilterPermutation.R 97.22% <97.22%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52748f8...f5ffd9a. Read the comment docs.

@lintr-bot
Copy link

R/FilterPermutation.R:94:50: style: Commas should always have a space after.

​          task$cbind(column[sample(nrow(column)),])
                                                 ^

@be-marc be-marc requested a review from mllg April 15, 2020 19:54
@mllg
Copy link
Member

mllg commented Apr 15, 2020

On a second thought, please clone the task in each iteration. The current version (as requested by me) is even worse ;)

@pat-s pat-s marked this pull request as draft April 17, 2020 07:47
@pat-s
Copy link
Member

pat-s commented Jul 17, 2020

@be-marc Is this stalled or do you plan to proceed here?

@be-marc be-marc marked this pull request as ready for review July 17, 2020 12:14
@be-marc
Copy link
Member Author

be-marc commented Jul 17, 2020

This might be ready to merge. @mllg ?

@mllg mllg merged commit bd9831b into master Jul 17, 2020
@mllg mllg deleted the permutation branch July 17, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mllg mllg Awaiting requested review from mllg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@be-marc @codecov-io @lintr-bot @mllg @pat-s