Skip to content

chore: Remove upper bound on dask dependency #3343

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.

Sign up for GitHub

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

Jump to bottom
Merged
kevjumba merged 1 commit into from
Nov 28, 2022
Merged

chore: Remove upper bound on dask dependency #3343

kevjumba merged 1 commit into from
Nov 28, 2022

Conversation

@oliverholworthy
Copy link
Contributor

@oliverholworthy oliverholworthy commented Nov 9, 2022

What this PR does / why we need it:

Motivation: Being able to use feast in an application with more recent versions of dask in a Python 3.8+ environment without having to work around conflicts due to versions specified in package.

This PR removes the upper bound on dask requirement added in #2342 This upper bound was added because newer versions of dask dropped support for Python 3.7.

However, the upper bound isn't necesesary for Python 3.7 compatibility (unless a version of dask was released that had the wrong python_requires specified.)

  • pip install dask (or "dask>=2021.*") in a Python 3.7 environment installs dask==2022.2.0 which was the last version that specfied comaptitibility with Python 3.7 (with python_requires in the pacakge config)

Additional context

It seems that in practice feast has dropped support for Python 3.7:

@oliverholworthy
Remove upper bound on dask dependency
159ed80 
Signed-off-by: Oliver Holworthy <oholworthy@nvidia.com>
@beauchbum
Copy link
Contributor

beauchbum commented Nov 10, 2022

Just bumped into this conflict today when trying to add feast as a dependency in our model-training repo. Thanks for opening a PR to address this.

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2022 •
edited
Loading

Codecov Report

Base: 67.65% // Head: 57.88% // Decreases project coverage by -9.77% ⚠️

Coverage data is based on head (159ed80) compared to base (5c9b6fe).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3343      +/-   ##
==========================================
- Coverage   67.65%   57.88%   -9.78%     
==========================================
  Files         181      215      +34     
  Lines       16645    18092    +1447     
==========================================
- Hits        11262    10472     -790     
- Misses       5383     7620    +2237
Flag Coverage Δ
integrationtests ?
unittests 57.88% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
setup.py 0.00% <ø> (ø)
...sts/integration/registration/test_universal_cli.py 20.20% <0.00%> (-79.80%) ⬇️
...ts/integration/offline_store/test_offline_write.py 26.08% <0.00%> (-73.92%) ⬇️
...fline_store/test_universal_historical_retrieval.py 28.75% <0.00%> (-71.25%) ⬇️
...dk/python/tests/integration/e2e/test_validation.py 26.71% <0.00%> (-70.23%) ⬇️
...ests/integration/e2e/test_python_feature_server.py 31.34% <0.00%> (-68.66%) ⬇️
...s/integration/registration/test_universal_types.py 32.25% <0.00%> (-67.75%) ⬇️
sdk/python/feast/infra/online_stores/redis.py 28.39% <0.00%> (-66.67%) ⬇️
sdk/python/tests/integration/e2e/test_usage_e2e.py 33.87% <0.00%> (-66.13%) ⬇️
sdk/python/tests/data/data_creator.py 34.78% <0.00%> (-65.22%) ⬇️
... and 168 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@achals
Copy link
Member

achals commented Nov 14, 2022

/ok-to-test

@feast-ci-bot
Copy link
Collaborator

feast-ci-bot commented Nov 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achals, oliverholworthy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@achals achals added the lgtm label Nov 28, 2022
@kevjumba kevjumba merged commit 2733368 into feast-dev:master Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@achals achals achals approved these changes

Assignees

No one assigned

Labels

approved lgtm ok-to-test size/XS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@oliverholworthy @beauchbum @codecov-commenter @achals @feast-ci-bot @kevjumba