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

Remove as_dir=False option from EntitySet.to_pickle() #20

Merged
merged 5 commits into from Oct 31, 2017

Conversation

Projects
None yet
3 participants
@bschreck
Contributor

bschreck commented Oct 23, 2017

EntitySet used to have two options for saving to disk

  • as_dir=False was supposed to save to a single pickle file. This was broken.
  • as_dir=True saved to a directory containing several pickle files of metadata about each Entity, and CSV files with the data for each Entity.

This PR removes the as_dir=False option.

@codecov-io

This comment has been minimized.

codecov-io commented Oct 23, 2017

Codecov Report

Merging #20 into master will increase coverage by 0.83%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage   86.83%   87.67%   +0.83%     
==========================================
  Files          73       73              
  Lines        6768     6773       +5     
==========================================
+ Hits         5877     5938      +61     
+ Misses        891      835      -56
Impacted Files Coverage Δ
featuretools/entityset/serialization.py 96.77% <100%> (+77.93%) ⬆️
featuretools/entityset/entityset.py 87.52% <100%> (+0.68%) ⬆️
featuretools/tests/entityset_tests/test_es.py 96.36% <92.3%> (-0.58%) ⬇️

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 fb87937...d5558f9. Read the comment docs.

shutil.rmtree(path)
es.to_pickle(path)
new_es = EntitySet.read_pickle(path)
assert es == new_es

This comment has been minimized.

@kmax12

kmax12 Oct 23, 2017

Member

fyi, i think this check only looks to make sure the entity set ids match.

This comment has been minimized.

@bschreck

bschreck Oct 23, 2017

Contributor

Whoops. Changed to __eq__(deep=True)

@@ -710,6 +710,8 @@ def normalize_entity(self, base_entity_id, new_entity_id, index,
transfer_types = {}
transfer_types[new_index] = type(base_entity[index])
for v in additional_variables + copy_variables:

This comment has been minimized.

@kmax12

kmax12 Oct 24, 2017

Member

Is this bug fix for us currently not transferring the type of additional_variables and copy_variables?

bschreck added some commits Oct 31, 2017

@kmax12

This comment has been minimized.

Member

kmax12 commented Oct 31, 2017

Looks good to me

@kmax12 kmax12 merged commit 2b75617 into master Oct 31, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@kmax12 kmax12 deleted the entityset-pickle-fix branch Dec 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment