Skip to content

Add a DeprecationWarning about deprecating the list API for ComponentGraphs#2488

Merged
angela97lin merged 4 commits intomainfrom
2483_list_API_deprecation_warning
Jul 9, 2021
Merged

Add a DeprecationWarning about deprecating the list API for ComponentGraphs#2488
angela97lin merged 4 commits intomainfrom
2483_list_API_deprecation_warning

Conversation

@angela97lin
Copy link
Contributor

@angela97lin angela97lin commented Jul 9, 2021

Closes #2483

@angela97lin angela97lin self-assigned this Jul 9, 2021
@codecov
Copy link

codecov bot commented Jul 9, 2021 •
edited
Loading

Codecov Report

Merging #2488 (1b48ff1) into main (f5b37b4) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2488     +/-   ##
=======================================
+ Coverage   99.7%   99.7%   +0.1%     
=======================================
  Files        283     283             
  Lines      25571   25585     +14     
=======================================
+ Hits       25469   25483     +14     
  Misses       102     102
Impacted Files Coverage Δ
evalml/pipelines/component_graph.py 99.4% <100.0%> (+0.1%) ⬆️
evalml/pipelines/pipeline_base.py 99.7% <100.0%> (+0.1%) ⬆️
...valml/tests/pipeline_tests/test_component_graph.py 100.0% <100.0%> (ø)

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 f5b37b4...1b48ff1. Read the comment docs.

angela97lin
angela97lin commented Jul 9, 2021
View reviewed changes
evalml/pipelines/pipeline_base.py
self.component_graph = ComponentGraph().from_list(
component_graph, random_seed=self.random_seed
)
with warnings.catch_warnings():
Copy link
Contributor Author

@angela97lin angela97lin Jul 9, 2021

Choose a reason for hiding this comment

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

I added this here to filter for the DeprecationWarning because users who are using AutoMLSearch or PipelineBase might be confused as to why they're receiving this. I think the warning should only surface if the user directly initializes a ComponentGraph using a list directly, Pipelines should still handle lists of components :)

Copy link
Contributor

@dsherry dsherry Jul 9, 2021

Choose a reason for hiding this comment

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

Great call

@angela97lin angela97lin marked this pull request as ready for review July 9, 2021 20:43
jeremyliweishih
jeremyliweishih approved these changes Jul 9, 2021
View reviewed changes
Copy link
Collaborator

@jeremyliweishih jeremyliweishih left a comment

Choose a reason for hiding this comment

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

nice! I like the warning catch.

dsherry
dsherry approved these changes Jul 9, 2021
View reviewed changes
Copy link
Contributor

@dsherry dsherry left a comment

Choose a reason for hiding this comment

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

🚢 😁

evalml/pipelines/pipeline_base.py
self.component_graph = ComponentGraph().from_list(
component_graph, random_seed=self.random_seed
)
with warnings.catch_warnings():
Copy link
Contributor

@dsherry dsherry Jul 9, 2021

Choose a reason for hiding this comment

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

Great call

@angela97lin angela97lin merged commit bd6bae6 into main Jul 9, 2021
@angela97lin angela97lin deleted the 2483_list_API_deprecation_warning branch July 9, 2021 21:33
@chukarsten chukarsten mentioned this pull request Jul 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeremyliweishih jeremyliweishih jeremyliweishih approved these changes

@chukarsten chukarsten Awaiting requested review from chukarsten

@freddyaboulton freddyaboulton Awaiting requested review from freddyaboulton

@bchen1116 bchen1116 Awaiting requested review from bchen1116

@christopherbunn christopherbunn Awaiting requested review from christopherbunn

+1 more reviewer

@dsherry dsherry dsherry approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@angela97lin angela97lin

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a DeprecationWarning about deprecating the list API for ComponentGraphs

3 participants

@angela97lin @dsherry @jeremyliweishih