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
Allow passing entities without relationships to CFM #1290
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1290 +/- ##
==========================================
+ Coverage 98.36% 98.38% +0.01%
==========================================
Files 134 134
Lines 14456 14473 +17
==========================================
+ Hits 14220 14239 +19
+ Misses 236 234 -2
Continue to review full report at Codecov.
|
@@ -142,8 +142,7 @@ def calculate_feature_matrix(features, entityset=None, cutoff_time=None, instanc | |||
# handle loading entityset | |||
from featuretools.entityset.entityset import EntitySet | |||
if not isinstance(entityset, EntitySet): | |||
if entities is not None and relationships is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering--why not keep the check for whether entities is not None here? Is there a world in which having no entities wouldn't end up erroring?
If not, it might be nice to catch this with its own error instead of Entity transactions does not exist in entityset
, which is where it gets caught in the test below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, if we catch there we can give a more descriptive error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Fixes #1287
Before,
calculate_feature_matrix
would check that bothentities
andrelationships
were notNone
before creating an entityset with them, which required passing an empty iterable of relationships in order to work with a single entity. This PR changes that by removing that check and relying on theEntitySet
init method to handle valdiating theentities
andrelationships