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

A simple fix for occasional errors when using sffs #2486

Merged
merged 16 commits into from Mar 18, 2019

Conversation

4 participants
@bmihaljevic
Copy link
Contributor

commented Nov 14, 2018

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

bmihaljevic
@pat-s

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2018

More importantly : doesn't this need a test?

bmihaljevic and others added some commits Nov 16, 2018

bmihaljevic
@bmihaljevic

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2018

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

bmihaljevic and others added some commits Nov 20, 2018

@bmihaljevic

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2018

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

bmihaljevic added some commits Dec 3, 2018

@pat-s

This comment has been minimized.

Copy link
Member

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.

larskotthoff and others added some commits Mar 18, 2019

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

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@larskotthoff

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

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

@pat-s

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

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

@larskotthoff

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

Ok, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.