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

Closed
wants to merge 1 commit into 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
@SauravMaheshkar
Copy link
Collaborator

How does #1559 affect this ?

@guarin
Copy link
Contributor

guarin commented Jul 10, 2024

@MalteEbner what is the state of this PR?

IMO there are two options:

  • Handle from numpy.typing import NDArray depending on whether it is available or not. If it is not available we could define a generic alias for NDArray instead. Not sure how that would work for older Python versions though.
  • Increase minimal numpy version in pyproject.toml. Numpy 1.18 has reached end of life >2 years ago.

The current state with numpy>=1.18.1, <2 as default requirement and numpy==1.26.6 as minimal requirement is not ideal.

@guarin
Copy link
Contributor

guarin commented Aug 8, 2024

This has been fixed in #1625

We still support numpy<1.20 and only import from numpy.typing when type-checking.

@guarin guarin closed this Aug 8, 2024
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.

5 participants