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

Clean up unused methods #293

Merged
merged 9 commits into from
Oct 26, 2018
Merged

Clean up unused methods #293

merged 9 commits into from
Oct 26, 2018

Conversation

kmax12
Copy link
Contributor

@kmax12 kmax12 commented Oct 25, 2018

Made minor changes that clean up the code. No api changes.

Changes

  • removed Relationship.get_other_variable because the method wasn't used
  • removed Relationship.get_other_entity because it was only used one. Added logic to Entity.related_instances instead
  • removed Relationship.get_entity_variable because it was only used one. Added logic to Entity.related_instances instead
  • removed Relationship._get_link_variable_name and EntitySet. gen_relationship_var because they did the same thing and it isn't really a method of either of those classes. Created method get_relationship_variable_id in our utils file instead.
  • removed unused methods _check_variable_list, _check_variable , and _check_entity from wrangle.py
  • remove training_window_is_dict handling in related instances that is not possible
  • removed unused methods Entity.is_child_of and Entity.is_parent_of
  • remove unused method EntitySet.get_relationships
  • remove unused method make_index_variable_name
  • remove unused class RedirectStdStreams
  • remove invalid property Discrete.percent_unique. this property access other properties that are no longer defined on Variable

@kmax12 kmax12 requested a review from rwedge October 25, 2018 20:13
@kmax12 kmax12 changed the title Clean up Relationship class Clean up unusedmethods Oct 25, 2018
@kmax12 kmax12 changed the title Clean up unusedmethods Clean up unused methods Oct 25, 2018
@rwedge
Copy link
Contributor

rwedge commented Oct 26, 2018

Did the training_window_is_dict change make it into the PR?

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@c7bf678). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #293   +/-   ##
=========================================
  Coverage          ?   94.73%           
=========================================
  Files             ?       71           
  Lines             ?     7668           
  Branches          ?        0           
=========================================
  Hits              ?     7264           
  Misses            ?      404           
  Partials          ?        0
Impacted Files Coverage Δ
featuretools/utils/wrangle.py 79.77% <ø> (ø)
featuretools/entityset/relationship.py 95.23% <ø> (ø)
...turetools/computational_backends/pandas_backend.py 94.11% <100%> (ø)
featuretools/entityset/entityset.py 93.75% <100%> (ø)
featuretools/utils/gen_utils.py 74.13% <100%> (ø)
...computational_backends/calculate_feature_matrix.py 96.96% <100%> (ø)

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 c7bf678...021f18f. Read the comment docs.

@kmax12
Copy link
Contributor Author

kmax12 commented Oct 26, 2018

you're right, it didn't. just fixed

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.

Looks good!

@kmax12 kmax12 merged commit b5c0b5e into master Oct 26, 2018
@rwedge rwedge mentioned this pull request Oct 31, 2018
@kmax12 kmax12 deleted the clean-up-relationship-class branch December 1, 2018 16:49
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.

3 participants