-
Notifications
You must be signed in to change notification settings - Fork 16
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 TF version, remove py3.5 support #105
Conversation
7e1a312
to
910a06f
Compare
910a06f
to
c88c03b
Compare
@@ -1,4 +1,4 @@ | |||
numpy~=1.14 | |||
scipy~=1.0 | |||
scikit-learn>=0.19 | |||
tensorflow>=1.15.2,<2 | |||
scikit-learn>=0.20.0,<0.23.0 |
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.
Using 0.19.x and 0.23.x makes the tests fail. I'm not sure how we didn't catch the 0.19.x incompatibility earlier, but I'd guess that the tests have been running as 0.20+ for a while.
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.
Changes look good, but in the top level comment you say
It also removes Python 3.5 support and adds Python 3.8 support.
but it looks like these changes don't mention Python 3.8
Ah, oops. I initially tried adding 3.8 support but ran into the problem that tensorflow 1.15.4 isn't available for python 3.8 (https://pypi.org/project/tensorflow/1.15.4/#files). I updated the PR description. |
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.
LGTM!
This updates the minimum TF version, per GHSA-4g9f-63rx-5cw4.
It also removes Python 3.5 support and sets more specific scikit-learn version requirements to avoid test failures and incompatibilities.