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

upgrade requirements/base.txt minimum numpy version to 1.20 for typing #1490

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MalteEbner
Copy link
Contributor

We are using the numpy.typing subpackage, which is only available in numpy >= 1.20: https://numpy.org/devdocs/reference/typing.html#module-numpy.typing

Thus we need to update the minimum requirement.

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6bd7553) 84.38% compared to head (3f1bc45) 84.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1490   +/-   ##
=======================================
  Coverage   84.38%   84.38%           
=======================================
  Files         139      139           
  Lines        5756     5756           
=======================================
  Hits         4857     4857           
  Misses        899      899           

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

Copy link
Contributor

@IgorSusmelj IgorSusmelj left a comment

Choose a reason for hiding this comment

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

Let's not merge this as of now. We need a proper strategy to deal with the dependencies.

For context:

  • We just added https://github.com/lightly-ai/lightly/blob/master/requirements/minimal_requirements.txt which contains the actual minimum requirements needed to use all the features. We have a new test for this in the CI pipeline
  • We did not want to change all the base requirements yet, as we have many users using only very specific features due to the modular way lightly works. We will have to figure out how we want to deal with that. The issue is that we accidentally broke many requirements because our previous tests were always installing latest packages. So we never tested if the requirements we once established are still valid or not.

Copy link
Contributor

@japrescott japrescott left a comment

Choose a reason for hiding this comment

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

lets quickly review the minimum requirements

@MalteEbner MalteEbner marked this pull request as draft January 30, 2024 16:13
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.

None yet

3 participants