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.

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

n.show argument in plotFilterValues ignored #2689

Closed
albersonmiranda opened this issue Nov 26, 2019 · 4 comments · Fixed by #2696
Closed

n.show argument in plotFilterValues ignored #2689

albersonmiranda opened this issue Nov 26, 2019 · 4 comments · Fixed by #2696
Labels

Comments

@albersonmiranda
Copy link
Contributor

albersonmiranda commented Nov 26, 2019

I'm working on a data set with several features and just noticed plotFilterValues is ignoring n.show argument. Code inspected, couldn't find how it should work.

I ran something like
importance = generateFilterValuesData(train.task, method = "FSelectorRcpp_information.gain") plotFilterValues(importance, n.show = 10)
and got all features, instead of 10.

Changed line 188 on the file for
data = fvalues$data %>% arrange(desc(value)) %>% slice(1:n.show)
and it seems to do the trick.

Could you guys check it?

Edit by @pat-s: Reprex added

library(mlr)
#> Loading required package: ParamHelpers

importance = generateFilterValuesData(spam.task, method = "FSelectorRcpp_information.gain")
plotFilterValues(importance, n.show = 10)

Created on 2019-12-03 by the reprex package (v0.3.0)

@larskotthoff
Copy link
Sponsor Member

Ah, yes, that is a bug. Fixing this will be trickier than the code you're proposing though as there's also an argument that allows to specify the order. This is only used to reorder values in the plot though, not to actually sort the data.

The documentation doesn't specify what values are selected...

I think it probably makes most sense here to sort the values, select the top n, and then plot in that order (and change the documentation accordingly). Other opinions @pat-s ?

@albersonmiranda
Copy link
Contributor Author

@larskotthoff when you say 'sort the values', you mean that sort argument would not reorder values in the plot, but actually sort data meaning to select top n most important or the top n least important features?
Or keep sort for the plot only and sort the data always in descending way, as i did above?

There could even be both sort and reorder arguments, one for data and another for the plot, with default descending for both (which makes more sense to me).

@larskotthoff
Copy link
Sponsor Member

I mean sort the data. That would make most sense here I think -- reordering it just for the plot doesn't make it as obvious what's happening.

@pat-s
Copy link
Member

pat-s commented Dec 3, 2019

@albersonmiranda Thanks. For an unknown reason, n.show was not used at all in plotFilterValues().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants