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

features_selection typo in base_automl.py #553

Closed
arvkevi opened this issue Jul 8, 2022 · 4 comments · Fixed by #555
Closed

features_selection typo in base_automl.py #553

arvkevi opened this issue Jul 8, 2022 · 4 comments · Fixed by #555
Assignees
@arvkevi
Labels
bug Something isn't working

Comments

@arvkevi
Copy link

arvkevi commented Jul 8, 2022

I'm wondering if there is a typo on this line, should "features_selectiom" be changed to "features_selection"?

I tried to write a unit test to catch this by fitting AutoML, then manually modifying params.json, and loading a new AutoML from file. But I could not get my debugger to step into this line, so figured I'd ask here before going any further. I'm not sure what workflow would trigger this being silently missed, it seems to have little to no impact.
@pplonski
Copy link
Contributor

pplonski commented Jul 8, 2022

@arvkevi you are right! It is a typo. Would you open a PR with fix?

To reproduce this bug you will need to:

  • start AutoML training with features_selection=True,
  • interrupt the AutoML training before feature selection,
  • load the AutoML and continue the training, because of the bug the feature selection will not be performed.

It might be hard to write unit test for that, but for sure this bug might be observed and might have impact.

Thank you for finding this and reporting!

@pplonski pplonski added the bug Something isn't working label Jul 8, 2022
@Vamp1899
Copy link

Hi , @pplonski i can give it a shot with PR ,but new to open source so might need your guidance a bit.

@pplonski
Copy link
Contributor

@Vamp1899 sure! Please follow steps:

  • just create a fork
  • do a commit with typo fix
  • do a PR to mljar-supervised repo

@pplonski
Copy link
Contributor

Fixed in #555

Thank you @arvkevi for finding the bug!

@pplonski pplonski linked a pull request Jul 18, 2022 that will close this issue
solved typo in base_model.py #555
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arvkevi arvkevi

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

solved typo in base_model.py hpoddar2810/mljar-supervised
3 participants
@pplonski @arvkevi @Vamp1899