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

Add verbose flag to top level search method #2813

Merged
merged 9 commits into from
Sep 20, 2021
Merged

Add verbose flag to top level search method #2813

merged 9 commits into from
Sep 20, 2021

Conversation

jeremyliweishih
Copy link
Collaborator

Fixes #2791.

@codecov
Copy link

codecov bot commented Sep 20, 2021 •
edited
Loading

Codecov Report

Merging #2813 (a129a0b) into main (af994b3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2813   +/-   ##
=====================================
  Coverage   99.8%   99.8%           
=====================================
  Files        297     297           
  Lines      27755   27755           
=====================================
  Hits       27687   27687           
  Misses        68      68
Impacted Files Coverage Δ
evalml/automl/automl_search.py 99.9% <ø> (ø)

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 af994b3...a129a0b. Read the comment docs.

freddyaboulton
freddyaboulton approved these changes Sep 20, 2021
View reviewed changes
Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thanks @jeremyliweishih ! I think this is good to merge but I'd like your input on these two points beforehand:

  1. Should we add test coverage?
  2. Why not also make the change for search_iterative ?

evalml/automl/automl_search.py Outdated
from sklearn.model_selection import BaseCrossValidator

from dask import distributed as dd
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a typo right?

@jeremyliweishih
Copy link
Collaborator Author

@freddyaboulton search_iterative takes in **kwargs so it doesn't need this explicit flag. I'll fix the typo but I think the coverage in test_automl.py is enough for the verbose flag.

@angela97lin angela97lin self-requested a review September 20, 2021 21:08
angela97lin
angela97lin approved these changes Sep 20, 2021
View reviewed changes
Copy link
Contributor

@angela97lin angela97lin left a comment

Choose a reason for hiding this comment

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

LGTM!

@jeremyliweishih jeremyliweishih merged commit 4d55cb1 into main Sep 20, 2021
@chukarsten chukarsten mentioned this pull request Oct 1, 2021
Merged
@freddyaboulton freddyaboulton deleted the js_search_verbose branch May 13, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angela97lin angela97lin angela97lin approved these changes

@freddyaboulton freddyaboulton freddyaboulton approved these changes

Assignees

@jeremyliweishih jeremyliweishih

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add verbose flag to search()
3 participants
@jeremyliweishih @angela97lin @freddyaboulton