-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
Compliancy with tf-nightly #9570
Conversation
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.
I wonder how many different names tensorflow can have ;-) Thanks for fixing!
src/transformers/file_utils.py
Outdated
if _tf_available: | ||
if version.parse(_tf_version) < version.parse("2"): | ||
if version.parse(_tf_version) < version.parse("2.3"): |
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.
No this was just an old check for version 1 that was left, but we should not hardcode anything here, as we will forget to update it when we pin tensorflow to another release.
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.
Should I let it as it is or completely remove this condition?
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.
I think we should leave it as is. Let's see what the others think!
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.
I agree with Sylvain that this should stay as it is, it's to indicate that the general framework detected (TensorFlow 1) will not work correctly as we expect another (TensorFlow 2).
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!
src/transformers/file_utils.py
Outdated
if _tf_available: | ||
if version.parse(_tf_version) < version.parse("2"): | ||
if version.parse(_tf_version) < version.parse("2.3"): |
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.
I agree with Sylvain that this should stay as it is, it's to indicate that the general framework detected (TensorFlow 1) will not work correctly as we expect another (TensorFlow 2).
Ok, just restored the previous version checking. |
try: | ||
_tf_version = importlib_metadata.version("tensorflow-gpu") | ||
except importlib_metadata.PackageNotFoundError: | ||
try: | ||
_tf_version = importlib_metadata.version("tf-nightly") | ||
except importlib_metadata.PackageNotFoundError: | ||
try: | ||
_tf_version = importlib_metadata.version("tf-nightly-cpu") | ||
except importlib_metadata.PackageNotFoundError: | ||
try: | ||
_tf_version = importlib_metadata.version("tf-nightly-gpu") | ||
except importlib_metadata.PackageNotFoundError: | ||
_tf_version = None | ||
_tf_available = False |
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.
Wow!
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.
Yeah, just done a quick search on pypi to be sure we have all of them 😄
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.
Might refactor this using a list of names at this stage ;-)
* Compliancy with tf-nightly * Add more version + restore min version check
* Compliancy with tf-nightly * Add more version + restore min version check
What does this PR do?
This PR makes the lib able to be used with the nightly builds of TensorFlow. And fix an issue with the min TensorFlow version.