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
Fixed deprecated access of collections members. #26173
Conversation
Closes #26095 |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
If there are problems importing
|
@NeilGirdhar can you change the pull request to do your proposed workaround for older python versions? TensorFlow still has to support python 2.7 and 3.5 |
@alextp can we rely on six if they push through this pull request? benjaminp/six#241 3.5 is no problem. collections.abc exists there. |
Yes, relying on six would be ideal.
We can also put this in
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/util/compat.py
and import from there
…On Mon, Mar 4, 2019 at 12:06 PM Neil ***@***.***> wrote:
@alextp <https://github.com/alextp> can we rely on six if they push
through this pull request? benjaminp/six#241
<benjaminp/six#241>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26173 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAATxb4uSavXm7V2iYroJq5jW8jkr19Cks5vTXzVgaJpZM4bUt-U>
.
--
- Alex
|
I don't understand these build errors. It seems to be totally unrelated to my change. Any ideas? |
@rthadur Could these errors have something to do with how the tests were run? |
@NeilGirdhar i believe you need to update the test cases which are associated with the files. |
@rthadur I don't see that? I see errors like "Broken by missing target @com_google_absl//absl/base:base" ? I don't see how this relates... |
@alextp any thoughts why this tests are failing ? |
@rthadur Looked like a tool failure, retrying |
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.
can you please above error
@@ -72,6 +71,8 @@ def testXXX(self): | |||
from google.protobuf import message | |||
from google.protobuf import text_format | |||
|
|||
from tensorflow.python.util.compat import collections_abc | |||
|
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.
tensorflow/python/util/protobuf/compare.py", line 75, in
from tensorflow.python.util.compat import collections_abc
ImportError: No module named tensorflow.python.util.compat
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've replaced this line with a relative import, and fixed some merge conflicts.
Changed the access of the abstract base classes to use a collections_abc compatibility stub. * This prevents warnings in Python 3.7, and runtime errors in Python 3.8+. * The compatibility stub bridges the gap between Python 3.3+ where collections.abc was factored out of collections.
PiperOrigin-RevId: 260814539
Changed the access of the abstract base classes to use collections.abc.
This prevents warnings in Python 3.7, and runtime errors in Python 3.8+.