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

Keep non-feature columns when encoding feature matrix #111

merged 5 commits into from Mar 14, 2018


Copy link

@rwedge rwedge commented Mar 14, 2018

#78 added the option of including non-feature columns with the calculated feature matrix, but encode_features was not including them when creating an encoded feature matrix. This PR fixes encode_features so it includes non-feature columns in the encoded feature matrix.

Copy link

codecov-io commented Mar 14, 2018

Codecov Report

Merging #111 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #111      +/-   ##
+ Coverage   88.24%   88.3%   +0.05%     
  Files          73      73              
  Lines        7412    7447      +35     
+ Hits         6541    6576      +35     
  Misses        871     871
Impacted Files Coverage Δ
...ests/computational_backend/ 100% <100%> (ø) ⬆️
featuretools/synthesis/ 94.54% <100%> (+0.92%) ⬆️

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 6c9a011...3116bb4. Read the comment docs.

@@ -65,6 +65,8 @@ def encode_features(feature_matrix, features, top_n=10, include_unknown=True,
X = feature_matrix.copy()

encoded = []
feature_names = [f.get_name() for f in features]
extra_columns = [col for col in X.columns if col not in feature_names]
Copy link

Choose a reason for hiding this comment

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

to make this safer, can we add in a check that all the features you pass in are columns in the feature matrix? with this change, we might silence issues where the feature matrix and features don't match

Copy link

kmax12 commented Mar 14, 2018

looks good to me. merging in.

@kmax12 kmax12 merged commit 959069d into master Mar 14, 2018
@rwedge rwedge mentioned this pull request Mar 21, 2018
@rwedge rwedge deleted the pass_extra_when_encoding branch June 3, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants