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

Update install dependencies #1476

Merged
merged 28 commits into from
Jan 16, 2024
Merged

Conversation

IgorSusmelj
Copy link
Contributor

@IgorSusmelj IgorSusmelj commented Jan 12, 2024

Changes

  • removes openapi.txt requirements file
  • move openapi dependencies to base.txt
  • use restore keys (apparently this should speed up the tests as we can use the cache from the master branch when creating PRs!?)
  • added a test to check minimal dependencies so we can track whenever we add new things that require version updates
  • update the readme and remove support for Python 3.6

This allows to install the lightly package with minimal dependencies (no PyTorch and torchvision)

New minimal dependencies (to run all the unittests) are the following:

pytorch_lightning==1.7.1
torch==1.11.0
torchvision==0.12.0

Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2b215aa) 85.50% compared to head (3d173f6) 85.50%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1476   +/-   ##
=======================================
  Coverage   85.50%   85.50%           
=======================================
  Files         135      135           
  Lines        5657     5657           
=======================================
  Hits         4837     4837           
  Misses        820      820           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@guarin guarin left a comment

Choose a reason for hiding this comment

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

LGTM!

@IgorSusmelj IgorSusmelj requested a review from guarin January 15, 2024 09:22
Copy link
Contributor

@guarin guarin left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the changes!

- name: Install Minimal Dependencies
run: pip install -r requirements/minimal_requirements.txt
- name: Install Package Without Dependencies
run: pip install --no-deps .
- name: Run Tests
run: python -m pytest -s -v --runslow
run: |
export LIGHTLY_SERVER_LOCATION="localhost:-1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary?

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 use it in the other test as well. To make sure we can run all tests offline and don't require the lightly api online.

@IgorSusmelj IgorSusmelj merged commit 8d3c1a9 into master Jan 16, 2024
12 checks passed
@IgorSusmelj IgorSusmelj deleted the update-install-dependencies-is branch January 16, 2024 12:32
guarin pushed a commit that referenced this pull request Jan 19, 2024
* remove openapi.txt requirements file
* Add pydantic and aenum to base.txt dependencies
* Add minimal dependencies requirements file
* Add test for minimal dependency
* Update readme and other docs to make it clear users need Python 3.7 to use lightly
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.

2 participants