Add relationship_path to agg and direct features #544
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #544 +/- ##
==========================================
+ Coverage 96.35% 96.46% +0.11%
==========================================
Files 116 117 +1
Lines 9234 9511 +277
==========================================
+ Hits 8897 9175 +278
+ Misses 337 336 -1
Continue to review full report at Codecov.
|
CJStadler
force-pushed
the
feature-relationship-paths
branch
from
6c9b644 to
79a3e41
Compare
CJStadler
commented
May 16, 2019
This adds an optional constructor parameter, and stores the relationship_path on the feature. If no relationship_path is passed to the constructor we search for a path between the target entity and base entity. If none or more than one is found we raise an error. When there are more than one possible paths between entities the feature name will include the full path. Also add parent_name and child_name to Relationship. This does not update any other logic to use the paths, as this will be added in later commits.
Since we need to detect nodes to which there are multiple paths, we only want to stop recursion the second time we visit a node.
Comparing the entities directly is very expensive, and this was significantly slowing down dfs.
So that the caller can access it without needing to check the feature type.
CJStadler
force-pushed
the
feature-relationship-paths
branch
from
bb5d1c1 to
3d087ea
Compare
kmax12
suggested changes
May 21, 2019
featuretools/tests/computational_backend/test_pandas_backend.py
Outdated
Show resolved
Hide resolved
And add logic for detecting old version.
- Remove description_to_relationship and relationship_to_description. - Rename get_arguments to to_dictionary.
And the same for the backward version. The new methods are generators which yield each path. This allows us to stop searching if we only need one path. Note that we ignore paths with cycles, meaning that a path may be said to be unique if the only other paths contain cycles.
kmax12
suggested changes
May 28, 2019
This can be inferred from relationship_path, but we don't want to change the API.
Between the same entities.
This is no reason to override the parent method here.
These were previously overlooked, and untested.
kmax12
previously approved these changes
May 28, 2019
This is equivalent, but makes the flow clearer.
kmax12
approved these changes
May 29, 2019
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds an optional constructor parameter, and stores the
relationship_pathon the feature. If norelationship_pathis passed to the constructor we search for a path between the target entity and base entity. If none or more than one is found we raise an error. When there are more than one possible paths between entities the feature name will include the full path.Also add
parent_nameandchild_nametoRelationship.This does not update any other logic to use the paths, as this will be added in later commits.
See #543 for context.