Skip to content

Conversation

@nfelt
Copy link
Contributor

@nfelt nfelt commented Mar 2, 2020

This puts a nice 🩹 over the deprecation warning spam emitted to the console by our usage of tf.compat.v1.io.tf_record_iterator (which we now do for new TF versions, as of #3185).

Actually migrating to something not deprecated is still tracked in #1711.

@nfelt nfelt requested a review from wchargin March 2, 2020 23:43
Comment on lines 44 to 49
try:
# Learn this one weird trick to make TF deprecation warnings go away.
from tensorflow.python.util import deprecation
return deprecation.silence()
except (ImportError, AttributeError):
return _NULL_CONTEXT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could avoid the _NULL_CONTEXT backport with something like this:

@contextlib.contextmanager
def _silence_deprecation_warnings():
  try:
    # Learn this one weird trick to make TF deprecation warnings go away.
    from tensorflow.python.util import deprecation
    cm = deprecation.silence()
  except (ImportError, AttributeError):
    yield
  else:
    with cm:
      yield

(Or cm = None; ...; if cm is not None: ... if you dislike the else.)

If you prefer it as is, then nit: nullcontext, not null_context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed spelling. IMO it's actually simpler and more readable just to use the null context manager and have an either-or rather than nested context managers?

Especially given that it's essentially no code to backport (I didn't even realize there was a stdlib one in any version until I checked on a whim).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay; fine with me if you prefer this.

@wchargin
Copy link
Contributor

wchargin commented Mar 3, 2020

Also, wow: my Noto doesn’t have U+1FA79. I guess there’s still some Tofu
left after all.

Comment on lines 44 to 49
try:
# Learn this one weird trick to make TF deprecation warnings go away.
from tensorflow.python.util import deprecation
return deprecation.silence()
except (ImportError, AttributeError):
return _NULL_CONTEXT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay; fine with me if you prefer this.

@nfelt nfelt merged commit ccd6fde into tensorflow:master Mar 3, 2020
@nfelt nfelt deleted the tfrecorditerator-deprecation-silence branch March 3, 2020 02:28
@nfelt nfelt mentioned this pull request Mar 3, 2020
bileschi pushed a commit to bileschi/tensorboard that referenced this pull request Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants