Skip to content

A simple fix for occasional errors when using sffs #2486

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

Merged
merged 16 commits into from
Mar 18, 2019

Conversation

bmihaljevic
Copy link
Contributor

@bmihaljevic bmihaljevic commented Nov 14, 2018

This is one-line fix for #2485. The issue is related to occasional crashing when using sffs feature selection.

@pat-s
Copy link
Member

pat-s commented Nov 14, 2018

@bmihaljevic Thanks for contributing.

  • Please edit your title so that it has a proper meaning
  • Reference the issue in the PR description
  • Do not simply copy/paste the PR template

@bmihaljevic bmihaljevic changed the title This fixes #2485 A simple fix for occasional errors when using sffs Nov 14, 2018
@berndbischl
Copy link
Member

More importantly : doesn't this need a test?

@bmihaljevic
Copy link
Contributor Author

I have just added a simple test that ensures that the minimal example from #2485 no longer raises an error.

@bmihaljevic
Copy link
Contributor Author

Hi,
I think I have addressed all suggested changes. Please let me know if I am missing something that is pending on my side.
Thanks,
Bojan

@pat-s
Copy link
Member

pat-s commented Mar 17, 2019

@larskotthoff Should we merge this? We now have a test. I don't know if the proposed change could break anything else but as long as the build is passing...

We should also add a NEWS entry.

@pat-s pat-s merged commit a6ac07e into mlr-org:master Mar 18, 2019
@larskotthoff
Copy link
Member

Did the travis build for this pass? It seemed to not want to start earlier.

@pat-s
Copy link
Member

pat-s commented Mar 18, 2019

Yours passed and I just added an NEWS entry, therefore I did not wait. Master is green.

@larskotthoff
Copy link
Member

Ok, thanks!

vrodriguezf pushed a commit to vrodriguezf/mlr that referenced this pull request Jan 16, 2021
* This fixes mlr-org#2485

* add a test for the mlr-org#2485 fix

* conform coding style and reduce test time

* Update test_featsel_selectFeaturesSequential.R

* NEWS for mlr-org#2486
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants