Skip to content

Conversation

@sbadithe
Copy link
Contributor

@sbadithe sbadithe commented Dec 13, 2022

  • We were looping over the input_features twice because of the call to _features_have_same_path, but I think one traversal should do the trick
  • Since the checks for being a DirectFeature and having the same path are so tightly coupled (this function is still doing exactly what it was before), I think it should be okay to move it into one function, but I'm open to ideas here
  • Helps reduce complexity, and we can delete the _features_have_same_path function
  • In _build_transform_features, removes function call and iterates once over the input_features. This reduces from three traversals to one (closes Reduce traversals over features in _build_transform_features #2402)

@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Merging #2400 (4b7b287) into main (84b7cd5) will decrease coverage by 0.00%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##             main    #2400      +/-   ##
==========================================
- Coverage   99.45%   99.44%   -0.01%     
==========================================
  Files         331      331              
  Lines       20425    20432       +7     
==========================================
+ Hits        20313    20319       +6     
- Misses        112      113       +1     
Impacted Files Coverage Δ
featuretools/synthesis/deep_feature_synthesis.py 99.56% <94.73%> (-0.22%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@sbadithe sbadithe self-assigned this Dec 13, 2022
@sbadithe sbadithe marked this pull request as ready for review December 13, 2022 06:45
@sbadithe sbadithe changed the title [DRAFT] Remove unnecessary traversal over input_features when checking path equality Remove unnecessary traversal over input_features when checking path equality Dec 13, 2022
@sbadithe sbadithe changed the title Remove unnecessary traversal over input_features when checking path equality Remove traversal over input_features when checking path equality Dec 13, 2022
@sbadithe sbadithe changed the title Remove traversal over input_features when checking path equality Remove extra traversal over input_features when checking path equality Dec 13, 2022
@gsheni gsheni requested a review from dvreed77 December 14, 2022 19:13
@sbadithe sbadithe changed the title Remove extra traversal over input_features when checking path equality Remove building groupby features if require_direct_input Dec 15, 2022
@sbadithe sbadithe changed the title Remove building groupby features if require_direct_input Refactor building groupby features if require_direct_input Dec 15, 2022
@sbadithe sbadithe requested a review from rwedge December 19, 2022 16:38
Copy link
Contributor

@rwedge rwedge left a comment

Choose a reason for hiding this comment

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

LGTM

@gsheni gsheni merged commit b3b3a9e into main Dec 20, 2022
@gsheni gsheni deleted the reduce-traversals-in-same-path-check branch December 20, 2022 16:42
@thehomebrewnerd thehomebrewnerd mentioned this pull request Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce traversals over features in _build_transform_features

5 participants