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

Jump to bottom

Fix ComponentGraph appending target to final_component_features if there is a component that returns both X and y #2358

Merged
merged 6 commits into from Jun 16, 2021
Merged

Fix ComponentGraph appending target to final_component_features if there is a component that returns both X and y #2358

merged 6 commits into from Jun 16, 2021

Conversation

angela97lin
Copy link
Contributor

Closes #2280

@angela97lin angela97lin self-assigned this Jun 9, 2021
@angela97lin angela97lin changed the title Fix ComponentGraph appends target to final_component_features if there is a component that returns both X and y Fix ComponentGraph appending target to final_component_features if there is a component that returns both X and y Jun 9, 2021
@codecov
Copy link

codecov bot commented Jun 10, 2021 •
edited

Codecov Report

Merging #2358 (ae2606c) into main (2e1d8ab) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2358     +/-   ##
=======================================
+ Coverage   99.7%   99.7%   +0.1%     
=======================================
  Files        281     281             
  Lines      24925   24934      +9     
=======================================
+ Hits       24828   24837      +9     
  Misses        97      97
Impacted Files Coverage Δ
evalml/pipelines/component_graph.py 99.4% <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 2e1d8ab...ae2606c. Read the comment docs.

angela97lin
angela97lin commented Jun 10, 2021
View reviewed changes
evalml/pipelines/component_graph.py

parent_inputs = [
parent_input
for parent_input in self.get_parents(self.compute_order[-1])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We default to component name --> pass X so just checking that we're not checking a .y parent input :)

@angela97lin angela97lin marked this pull request as ready for review June 10, 2021 19:41
jeremyliweishih
jeremyliweishih reviewed Jun 10, 2021
View reviewed changes
Copy link
Contributor

@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.

lgtm

bchen1116
bchen1116 approved these changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@bchen1116 bchen1116 left a comment

Choose a reason for hiding this comment

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

LGTM!

@angela97lin angela97lin merged commit af00017 into main Jun 16, 2021
@angela97lin angela97lin deleted the 2280_cg branch June 16, 2021 03:58
@chukarsten chukarsten mentioned this pull request Jun 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 left review comments

@bchen1116 bchen1116 bchen1116 approved these changes

@dsherry dsherry Awaiting requested review from dsherry

@chukarsten chukarsten Awaiting requested review from chukarsten

@freddyaboulton freddyaboulton Awaiting requested review from freddyaboulton

@ParthivNaresh ParthivNaresh Awaiting requested review from ParthivNaresh

Assignees

@angela97lin angela97lin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ComponentGraph appends target to final_component_features if there is a component that returns both X and y
3 participants
@angela97lin @jeremyliweishih @bchen1116