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

No EntitySet required in loading/saving features #141

Merged
merged 22 commits into from May 9, 2018
Merged

Conversation

bschreck
Copy link
Contributor

@bschreck bschreck commented May 8, 2018

By creating an EntitySet.metadata object, this removes the need to remove (and thus place back upon loading) the references to the EntitySet inside of a list of features before saving to disk. This also eliminates the need for any fanciness before/after pickling.

This also means that calculate_feature_matrix needs an explicit EntitySet passed in, rather than being able to access the referenced EntitySet from the feature objects.

The new API is like this:

ft.save_features(fl, "fl.p")
fl = ft.load_features("fl.p")
ft.calculate_feature_matrix(fl, entityset, **kwargs)

@codecov-io
Copy link

codecov-io commented May 9, 2018

Codecov Report

Merging #141 into master will decrease coverage by 0.83%.
The diff coverage is 90.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
- Coverage   92.21%   91.37%   -0.84%     
==========================================
  Files          72       72              
  Lines        7707     7722      +15     
==========================================
- Hits         7107     7056      -51     
- Misses        600      666      +66
Impacted Files Coverage Δ
.../feature_function_tests/test_transform_features.py 98.84% <ø> (-0.01%) ⬇️
featuretools/synthesis/dfs.py 100% <ø> (ø) ⬆️
...ols/tests/feature_function_tests/test_agg_feats.py 98.5% <ø> (-0.01%) ⬇️
featuretools/entityset/relationship.py 82.6% <ø> (ø) ⬆️
featuretools/variable_types/variable.py 91.66% <ø> (-0.7%) ⬇️
...aturetools/tests/entityset_tests/test_pandas_es.py 99.75% <ø> (-0.02%) ⬇️
featuretools/primitives/transform_primitive.py 86.29% <0%> (-11.49%) ⬇️
...utational_backend/test_calculate_feature_matrix.py 99.55% <100%> (ø) ⬆️
featuretools/synthesis/deep_feature_synthesis.py 92.6% <100%> (ø) ⬆️
...computational_backends/calculate_feature_matrix.py 98.2% <100%> (-0.39%) ⬇️
... and 13 more

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 dfc43a5...33a853a. Read the comment docs.

r.entityset = new_entityset
return new_entityset

# TODO make these private
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this todo


@property
def metadata(self):
'''
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a docstring to the first line of this so it shows up properly in the documentation?

return new_v

@property
def is_metadata(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

add docstring

@bschreck bschreck merged commit f3577d0 into master May 9, 2018
@bschreck bschreck deleted the fl-no-entityset branch May 9, 2018 18:28
@rwedge rwedge mentioned this pull request May 30, 2018
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.

None yet

3 participants