-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Don't pass a status to _reader.GetNext() #1086
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,8 @@ | |
from __future__ import division | ||
from __future__ import print_function | ||
|
||
import inspect | ||
|
||
import tensorflow as tf | ||
|
||
|
||
|
@@ -49,8 +51,12 @@ def Load(self): | |
tf.logging.debug('Loading events from %s', self._file_path) | ||
while True: | ||
try: | ||
with tf.errors.raise_exception_on_not_ok_status() as status: | ||
self._reader.GetNext(status) | ||
if len(inspect.getargspec(self._reader.GetNext).args) is 0: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also looks like pylint is complaining about getargspec being deprecated on py3.6+. For now probably fine to just suppress There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks yeah - I saw that - I also assumed the code works fine (with the "is 0" check) since it didn't complain about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it didn't complain because right now with existing tf-nightly len(args) == 2 so this branch isn't triggered. But when tf-nightly picks up your change, len(args) will be == 1 (since "self" will still count as an argument), so this check will still fail, even though the intent is that it should pass. So I think this should be |
||
self._reader.GetNext() | ||
else: | ||
# GetNext() expects a status argument on TF <= 1.7 | ||
with tf.errors.raise_exception_on_not_ok_status() as status: | ||
self._reader.GetNext(status) | ||
except (tf.errors.DataLossError, tf.errors.OutOfRangeError): | ||
# We ignore partial read exceptions, because a record may be truncated. | ||
# PyRecordReader holds the offset prior to the failed read, so 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.
Does checking for length 0 work? When i was looking at this, "self" is considered the first argument (even though it's a bound function):
Oops, went back to look at my example and I got the negation wrong, my bad. I should have said "not args[1:]" but at that point an explicit length check is probably clearer.