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

Moved dask, distributed imports #634

Merged
merged 13 commits into from Jul 8, 2019

Conversation

@gsheni
Copy link
Member

commented Jul 1, 2019

  • Moved dask, distributed imports so they are inside the function. This is because dask/distributed is an optional library for featuretools
gsheni added 3 commits Jul 1, 2019

@gsheni gsheni requested review from kmax12 and rwedge Jul 1, 2019

@gsheni gsheni self-assigned this Jul 1, 2019

gsheni added 2 commits Jul 2, 2019

@gsheni gsheni changed the title Fix dask imports Moved dask, distributed imports Jul 2, 2019

@codecov

This comment has been minimized.

Copy link

commented Jul 2, 2019

Codecov Report

Merging #634 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #634      +/-   ##
==========================================
+ Coverage   97.43%   97.43%   +<.01%     
==========================================
  Files         118      118              
  Lines        9535     9538       +3     
==========================================
+ Hits         9290     9293       +3     
  Misses        245      245
Impacted Files Coverage Δ
featuretools/computational_backends/utils.py 95.83% <100%> (+0.07%) ⬆️
featuretools/tests/testing_utils/__init__.py 100% <100%> (ø) ⬆️
featuretools/tests/testing_utils/cluster.py 100% <100%> (ø) ⬆️
...utational_backend/test_calculate_feature_matrix.py 99.35% <100%> (-0.01%) ⬇️

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 32d4b22...433a3b2. Read the comment docs.

@rwedge
Copy link
Contributor

left a comment

Can we move dask and distributed from requirements.txt to test-requirements.txt since they are now optional

gsheni added 2 commits Jul 2, 2019
@gsheni

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2019

Can we move dask and distributed from requirements.txt to test-requirements.txt since they are now optional

Let's keep it in requirements.txt so that if users run DFS with n_jobs > 1, it won't give an error.

gsheni added 2 commits Jul 4, 2019
@kmax12
Copy link
Member

left a comment

just a few comments. otherwise looks good to me

docs/source/changelog.rst Outdated Show resolved Hide resolved
gsheni added 2 commits Jul 8, 2019

@gsheni gsheni requested review from kmax12 and rwedge Jul 8, 2019

@kmax12
kmax12 approved these changes Jul 8, 2019

@gsheni gsheni removed the request for review from rwedge Jul 8, 2019

Answered here #634 (comment)

@gsheni gsheni merged commit a990858 into master Jul 8, 2019

4 checks passed

codecov/patch 100% of diff hit (target 97.43%)
Details
codecov/project 97.43% (+<.01%) compared to 32d4b22
Details
license/cla Contributor License Agreement is signed.
Details
test_all_python_versions Workflow: test_all_python_versions
Details

@gsheni gsheni deleted the fix_dask_imports branch Jul 8, 2019

@rwedge rwedge referenced this pull request Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.