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

Normalize entity same index as base entity #681

Merged
merged 6 commits into from Jul 22, 2019

Conversation

@jeremyliweishih
Copy link
Contributor

commented Jul 22, 2019

This request addresses #670 .

Adds a check and raises an exception if the index of the base entity is equal to the index argument. This request adds an accompanying test case.

As stated in #670, Feature tools does not handle one-to-one relationships in entitysets. Therefore, this check must be made to stop creating a one-to-one relationship with the same index.

@jeremyliweishih jeremyliweishih changed the title Normalize entity same Normalize entity same index as base entity Jul 22, 2019

@CLAassistant

This comment has been minimized.

Copy link

commented Jul 22, 2019

CLA assistant check
All committers have signed the CLA.

@jeremyliweishih jeremyliweishih requested review from rwedge and kmax12 Jul 22, 2019

@@ -11,6 +11,7 @@ Changelog
* Set index after adding ancestor relationship variables (:pr:`668`)
* Fix user-supplied variable_types modification in Entity init (:pr:`675`)
* Don't calculate dependencies of unnecessary features (:pr:`667`)
* Normalize entity same index as base entity #681 (:pr:`681`)

This comment has been minimized.

Copy link
@rwedge

rwedge Jul 22, 2019

Contributor

The ":pr:681" syntax will create a link to this pr, you can remove the "#681". Also can you add a little more to the description so someone unfamiliar could grasp the issue?

@rwedge
rwedge approved these changes Jul 22, 2019
Copy link
Contributor

left a comment

Looks good!

@jeremyliweishih jeremyliweishih merged commit 6f4ffd7 into master Jul 22, 2019

2 checks passed

license/cla Contributor License Agreement is signed.
Details
test_all_python_versions Workflow: test_all_python_versions
Details
@codecov

This comment has been minimized.

Copy link

commented Jul 23, 2019

Codecov Report

Merging #681 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   97.44%   97.44%   +<.01%     
==========================================
  Files         118      118              
  Lines        9634     9643       +9     
==========================================
+ Hits         9388     9397       +9     
  Misses        246      246
Impacted Files Coverage Δ
featuretools/entityset/entityset.py 95.53% <100%> (+0.02%) ⬆️
featuretools/tests/entityset_tests/test_es.py 100% <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 1bc5f97...f5d9def. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.