Handle features with dataframe names that aren't 'X' #3873

tamargrey · 2022-12-05T18:51:45Z

If features are passed in, uses the dataframe_name from the Feature object to set the dastaframe_name in _make_entity_set instead of always calling it "X".

This allows us to run calculate feature matrix from the DFS transformer on Features with a dataframe name that is not "X".

tamargrey · 2022-12-05T18:54:16Z

evalml/pipelines/components/transformers/preprocessing/featuretools.py

@@ -28,6 +28,8 @@ def __init__(self, index="index", features=None, random_seed=0, **kwargs):
        self.index = index
        self.features = features
        self._passed_in_features = True if features else None
+        # If features are passed in, they'll have a dataframe_name we should utilize
+        self._dataframe_name = self.features[0].dataframe_name if self.features else "X"


The use of self.features[0] here assumes that all features were created from the same dataframe.

I didn't add a comment here explaining that, since it seems that assumption is true for the whole component (for example, in how we create the entityset in _make_entity_set), but I'd be happy to note that here if it would be useful.

let's add the note!

codecov · 2022-12-05T18:59:44Z

Codecov Report

Merging #3873 (f45370d) into main (d8f9c68) will increase coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main   #3873     +/-   ##
=======================================
+ Coverage   99.7%   99.7%   +0.1%     
=======================================
  Files        344     344             
  Lines      36191   36211     +20     
=======================================
+ Hits       36054   36074     +20     
  Misses       137     137

Impacted Files	Coverage Δ
...ponents/transformers/preprocessing/featuretools.py	`100.0% <100.0%> (ø)`
evalml/tests/component_tests/test_featuretools.py	`100.0% <100.0%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

tamargrey · 2022-12-05T19:02:42Z

evalml/pipelines/components/transformers/preprocessing/featuretools.py

@@ -36,16 +38,21 @@ def _make_entity_set(self, X):
        ft_es = EntitySet()
        # TODO: This delete was introduced for compatibility with Featuretools 1.0.0.  This should
        # be removed after Featuretools handles unnamed dataframes being passed to this function.
+        # But even then, we should still use any dataframe name that might be available from input features.


The way I see it, if a user has passed Feature objects in, then we do have a dataframe_name that we can use for X instead of X.ww.name (even if they're the same!), so this TODO explaining why we delete the woodwork schema does still apply.

I wanted to make sure that was clear, since I think it's a little less obvious now that we aren't fully hardcoding "X".

@jeremyliweishih added it 4c37408 - let me know if that makes sense to you!

jeremyliweishih

amazing!

jeremyliweishih · 2022-12-05T19:40:01Z

evalml/pipelines/components/transformers/preprocessing/featuretools.py

@@ -28,6 +28,8 @@ def __init__(self, index="index", features=None, random_seed=0, **kwargs):
        self.index = index
        self.features = features
        self._passed_in_features = True if features else None
+        # If features are passed in, they'll have a dataframe_name we should utilize
+        self._dataframe_name = self.features[0].dataframe_name if self.features else "X"


let's add the note!

jeremyliweishih · 2022-12-05T19:40:29Z

evalml/pipelines/components/transformers/preprocessing/featuretools.py

@@ -36,16 +38,21 @@ def _make_entity_set(self, X):
        ft_es = EntitySet()
        # TODO: This delete was introduced for compatibility with Featuretools 1.0.0.  This should
        # be removed after Featuretools handles unnamed dataframes being passed to this function.
+        # But even then, we should still use any dataframe name that might be available from input features.


thehomebrewnerd

One non-blocking suggestion, but looks good to me overall.

thehomebrewnerd · 2022-12-06T15:25:19Z

evalml/pipelines/components/transformers/preprocessing/featuretools.py

        if self.index not in X.columns:
            es = ft_es.add_dataframe(
                dataframe=X,
-                dataframe_name="X",
+                dataframe_name=self._dataframe_name,
                index=self.index,
                make_index=True,
            )
        else:
-            es = ft_es.add_dataframe(dataframe=X, dataframe_name="X", index=self.index)
+            es = ft_es.add_dataframe(
+                dataframe=X,
+                dataframe_name=self._dataframe_name,
+                index=self.index,
+            )


This is a bit of an unrelated nitpick, but I think this entire code block can be cleaned up to remove the if/else construct:

should_make_index = self.index not in X.columns es = ft_es.add_dataframe( dataframe=X, dataframe_name=self._dataframe_name, index=self.index, make_index=should_make_index, )

nice, changed

Handle features with dataframe names that aren't 'X'

1880295

auto-assign bot assigned tamargrey Dec 5, 2022

tamargrey commented Dec 5, 2022

View reviewed changes

Tamar Grey added 2 commits December 5, 2022 13:56

Explain how this change relates to TODO

ed72ea6

Add release note

7f845a5

tamargrey commented Dec 5, 2022

View reviewed changes

tamargrey requested review from chukarsten, jeremyliweishih and thehomebrewnerd December 5, 2022 19:03

jeremyliweishih approved these changes Dec 5, 2022

View reviewed changes

Tamar Grey added 2 commits December 5, 2022 14:59

Explain assumption around dataframe name

4c37408

Fix punctuation

13b17b2

tamargrey mentioned this pull request Dec 5, 2022

Update dfs transformer features arg to be empty list for fast mode #3875

Merged

Merge branch 'main' into fix-dfs-feature-df-names

81d6e77

chukarsten approved these changes Dec 6, 2022

View reviewed changes

eccabay approved these changes Dec 6, 2022

View reviewed changes

thehomebrewnerd approved these changes Dec 6, 2022

View reviewed changes

cleanup

f45370d

tamargrey merged commit 53a4912 into main Dec 6, 2022

tamargrey deleted the fix-dfs-feature-df-names branch December 6, 2022 16:49

christopherbunn mentioned this pull request Dec 8, 2022

Release v0.64.0 #3879

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle features with dataframe names that aren't 'X' #3873

Handle features with dataframe names that aren't 'X' #3873

tamargrey commented Dec 5, 2022

tamargrey Dec 5, 2022

jeremyliweishih Dec 5, 2022

codecov bot commented Dec 5, 2022 •

edited

Loading

tamargrey Dec 5, 2022

jeremyliweishih Dec 5, 2022

tamargrey Dec 5, 2022

jeremyliweishih left a comment

jeremyliweishih Dec 5, 2022

jeremyliweishih Dec 5, 2022

thehomebrewnerd left a comment •

edited

Loading

thehomebrewnerd Dec 6, 2022 •

edited

Loading

tamargrey Dec 6, 2022

Handle features with dataframe names that aren't 'X' #3873

Handle features with dataframe names that aren't 'X' #3873

Conversation

tamargrey commented Dec 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 5, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremyliweishih left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thehomebrewnerd left a comment • edited Loading

Choose a reason for hiding this comment

thehomebrewnerd Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 5, 2022 •

edited

Loading

thehomebrewnerd left a comment •

edited

Loading

thehomebrewnerd Dec 6, 2022 •

edited

Loading