Skip to content

Make Dask an optional dependency#2560

Merged
gsheni merged 18 commits intomainfrom
make-dask-optional
Apr 25, 2023
Merged

Make Dask an optional dependency#2560
gsheni merged 18 commits intomainfrom
make-dask-optional

Conversation

@thehomebrewnerd
Copy link
Contributor

Make Dask an optional dependency

Closes #1423

@thehomebrewnerd thehomebrewnerd self-assigned this Apr 24, 2023
@thehomebrewnerd thehomebrewnerd marked this pull request as draft April 24, 2023 21:31
@codecov
Copy link

codecov bot commented Apr 24, 2023

Codecov Report

Merging #2560 (b5aced3) into main (1e40a89) will decrease coverage by 0.01%.
The diff coverage is 99.23%.

❗ Current head b5aced3 differs from pull request most recent head 91974be. Consider uploading reports for the commit 91974be to get more accurate results

@@            Coverage Diff             @@
##             main    #2560      +/-   ##
==========================================
- Coverage   99.46%   99.46%   -0.01%     
==========================================
  Files         392      392              
  Lines       23153    23211      +58     
==========================================
+ Hits        23030    23087      +57     
- Misses        123      124       +1     
Impacted Files Coverage Δ
featuretools/tests/utils_tests/test_utils_info.py 100.00% <ø> (ø)
featuretools/computational_backends/utils.py 95.28% <88.88%> (-0.37%) ⬇️
...computational_backends/calculate_feature_matrix.py 100.00% <100.00%> (ø)
...s/computational_backends/feature_set_calculator.py 98.71% <100.00%> (ø)
featuretools/entityset/deserialize.py 100.00% <100.00%> (ø)
featuretools/entityset/entityset.py 99.22% <100.00%> (ø)
featuretools/feature_base/feature_base.py 98.15% <100.00%> (ø)
...s/primitives/standard/aggregation/all_primitive.py 100.00% <100.00%> (ø)
...s/primitives/standard/aggregation/any_primitive.py 100.00% <100.00%> (ø)
...etools/primitives/standard/aggregation/num_true.py 96.15% <100.00%> (ø)
... and 18 more

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

@thehomebrewnerd thehomebrewnerd marked this pull request as ready for review April 25, 2023 15:59
Copy link
Contributor

@dvreed77 dvreed77 left a comment

Choose a reason for hiding this comment

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

lgtm, but would feel better with another approval

@gsheni gsheni requested a review from sbadithe April 25, 2023 17:44
yield cluster
distributed = pytest.importorskip(
"distributed",
reason="Dask not installed, skipping",
Copy link
Contributor

@sbadithe sbadithe Apr 25, 2023

Choose a reason for hiding this comment

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

Would it be a good idea to have a Dask not installed error message saved instead of copying it for all Dask tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could do that, but the only people that ever see this message are developers, so I don't think it's critical that this message be the same everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. I was wondering more from a code cleanliness perspective.

@thehomebrewnerd thehomebrewnerd enabled auto-merge (squash) April 25, 2023 19:42
@gsheni gsheni disabled auto-merge April 25, 2023 20:41
@gsheni gsheni merged commit a211206 into main Apr 25, 2023
@gsheni gsheni deleted the make-dask-optional branch April 25, 2023 20:41
@dvreed77 dvreed77 mentioned this pull request Apr 27, 2023
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.

Make dask an optional dependency

5 participants