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

Remove max_hlevel from DFS #608

Merged
merged 4 commits into from Jun 19, 2019
Merged

Conversation

CJStadler
Copy link
Contributor

This was not exposed in the dfs method so it always used the default
value of 2, it was undocumented, and it was unclear if it provided any
value.

Also remove EntitySet.find_path as it is no longer used.

The base branch should be changed to master after #600 is merged.

This was not exposed in the dfs method so it always used the default
value of 2, it was undocumented, and it was unclear if it provided any
value.
This is no longer used.
@codecov
Copy link

codecov bot commented Jun 19, 2019

Codecov Report

Merging #608 into multipath-dfs-no-cycles will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@                     Coverage Diff                     @@
##           multipath-dfs-no-cycles     #608      +/-   ##
===========================================================
+ Coverage                    96.35%   96.38%   +0.03%     
===========================================================
  Files                          119      119              
  Lines                         9713     9605     -108     
===========================================================
- Hits                          9359     9258     -101     
+ Misses                         354      347       -7
Impacted Files Coverage Δ
...uretools/tests/entityset_tests/test_es_metadata.py 98.13% <ø> (-0.22%) ⬇️
featuretools/entityset/entityset.py 95.5% <ø> (+0.14%) ⬆️
...ols/tests/synthesis/test_deep_feature_synthesis.py 100% <ø> (ø) ⬆️
featuretools/synthesis/deep_feature_synthesis.py 95.85% <100%> (-0.44%) ⬇️
featuretools/__init__.py 65.51% <0%> (-1.15%) ⬇️
featuretools/utils/api.py 100% <0%> (ø) ⬆️
featuretools/__main__.py 0% <0%> (ø) ⬆️
...ools/tests/primitive_tests/test_primitive_utils.py 100% <0%> (ø) ⬆️
featuretools/tests/utils_tests/test_cli_utils.py 100% <0%> (ø) ⬆️
...retools/primitives/standard/transform_primitive.py 100% <0%> (ø) ⬆️
... and 4 more

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 c0d8b5f...3a49d8d. Read the comment docs.

@kmax12 kmax12 self-requested a review June 19, 2019 15:34
Copy link
Contributor

@kmax12 kmax12 left a comment

Choose a reason for hiding this comment

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

onces this is good. let's just merge it into multipath-dfs-no-cycles branch

if ((max_depth is None or self._get_depth(f) <= max_depth) and
(self.max_hlevel is None or
self._max_hlevel(f) <= self.max_hlevel)):
if max_depth is None or self._get_depth(f) <= max_depth:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can replace self._get_depth(f) with f.get_depth(stop_at=self.seed_features) and remove self._get_depth

@CJStadler CJStadler merged commit d1390d5 into multipath-dfs-no-cycles Jun 19, 2019
@CJStadler CJStadler deleted the remove-max-hlevel branch June 19, 2019 20:30
CJStadler added a commit that referenced this pull request Jun 21, 2019
For entitysets with multiple paths between two entities DFS will now
create features through all paths. Entitysets with cycles in the
directed graph are not supported.

- Use feature.unique_name instead of hash in dicts
- Change EntitySet.get_backward_entities  and get_forward_entities
  to yield paths.
- Add RelationshipPath which represents a directed path through
  relationships.
- Add RelationshipPath.entities
- Warn if dfs produces a feature multiple times
- Remove max_hlevel from DFS (#608).
- Remove EntitySet.find_path
@rwedge rwedge mentioned this pull request Jul 3, 2019
johnnyheineken pushed a commit to johnnyheineken/featuretools that referenced this pull request Jul 7, 2019
For entitysets with multiple paths between two entities DFS will now
create features through all paths. Entitysets with cycles in the
directed graph are not supported.

- Use feature.unique_name instead of hash in dicts
- Change EntitySet.get_backward_entities  and get_forward_entities
  to yield paths.
- Add RelationshipPath which represents a directed path through
  relationships.
- Add RelationshipPath.entities
- Warn if dfs produces a feature multiple times
- Remove max_hlevel from DFS (alteryx#608).
- Remove EntitySet.find_path
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.

None yet

2 participants