-
Notifications
You must be signed in to change notification settings - Fork 285
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
There was a problem hiding this 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.
There was a problem hiding this 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
How does #1559 affect this ? |
@MalteEbner what is the state of this PR? IMO there are two options:
The current state with |
This has been fixed in #1625 We still support numpy<1.20 and only import from numpy.typing when type-checking. |
We are using the
numpy.typing
subpackage, which is only available in numpy >= 1.20: https://numpy.org/devdocs/reference/typing.html#module-numpy.typingThus we need to update the minimum requirement.