From 9277bb73a926684d4346a56fec6c117873a9a84a Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 14 Dec 2017 11:12:52 -0800 Subject: [PATCH] Continue to allow flag access before explicit parse. Made tf.flags.FLAGS a wrapper of absl.flags.FLAGS, when the flag is access, parse flags implicitly with sys.argv if not yet. PiperOrigin-RevId: 179068530 --- tensorflow/python/BUILD | 5 ++- tensorflow/python/platform/flags.py | 55 +++++++++++++++++++++++ tensorflow/python/platform/flags_test.py | 57 +++++++++++++++++++++++- 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/tensorflow/python/BUILD b/tensorflow/python/BUILD index 4012197bce4..e77fba4a4cd 100644 --- a/tensorflow/python/BUILD +++ b/tensorflow/python/BUILD @@ -171,7 +171,10 @@ tf_py_test( name = "flags_test", size = "small", srcs = ["platform/flags_test.py"], - additional_deps = [":platform"], + additional_deps = [ + ":client_testlib", + ":platform", + ], ) tf_py_test( diff --git a/tensorflow/python/platform/flags.py b/tensorflow/python/platform/flags.py index abd6f3d8550..6225db77440 100644 --- a/tensorflow/python/platform/flags.py +++ b/tensorflow/python/platform/flags.py @@ -19,6 +19,7 @@ from __future__ import division from __future__ import print_function import logging as _logging +import sys as _sys # go/tf-wildcard-import from absl.flags import * # pylint: disable=wildcard-import @@ -59,6 +60,58 @@ def _wrap_define_function(original_function): return tf_decorator.make_decorator(original_function, wrapper) +class _FlagValuesWrapper(object): + """Wrapper class for absl.flags.FLAGS. + + The difference is that tf.flags.FLAGS implicitly parses flags with sys.argv + when accessing the FLAGS values before it's explicitly parsed, + while absl.flags.FLAGS raises an exception. + """ + + def __init__(self, flags_object): + self.__dict__['__wrapped'] = flags_object + + def __getattribute__(self, name): + if name == '__dict__': + return super(_FlagValuesWrapper, self).__getattribute__(name) + return self.__dict__['__wrapped'].__getattribute__(name) + + def __getattr__(self, name): + wrapped = self.__dict__['__wrapped'] + # To maintain backwards compatibility, implicitly parse flags when reading + # a flag. + if not wrapped.is_parsed(): + wrapped(_sys.argv) + return wrapped.__getattr__(name) + + def __setattr__(self, name, value): + return self.__dict__['__wrapped'].__setattr__(name, value) + + def __delattr__(self, name): + return self.__dict__['__wrapped'].__delattr__(name) + + def __dir__(self): + return self.__dict__['__wrapped'].__dir__() + + def __getitem__(self, name): + return self.__dict__['__wrapped'].__getitem__(name) + + def __setitem__(self, name, flag): + return self.__dict__['__wrapped'].__setitem__(name, flag) + + def __len__(self): + return self.__dict__['__wrapped'].__len__() + + def __iter__(self): + return self.__dict__['__wrapped'].__iter__() + + def __str__(self): + return self.__dict__['__wrapped'].__str__() + + def __call__(self, *args, **kwargs): + return self.__dict__['__wrapped'].__call__(*args, **kwargs) + + # pylint: disable=invalid-name,used-before-assignment # absl.flags APIs use `default` as the name of the default value argument. # Allow the following functions continue to accept `default_value`. @@ -68,3 +121,5 @@ DEFINE_bool = DEFINE_boolean DEFINE_float = _wrap_define_function(DEFINE_float) DEFINE_integer = _wrap_define_function(DEFINE_integer) # pylint: enable=invalid-name,used-before-assignment + +FLAGS = _FlagValuesWrapper(FLAGS) # pylint: disable=used-before-assignment diff --git a/tensorflow/python/platform/flags_test.py b/tensorflow/python/platform/flags_test.py index e8200142dd0..bd3c8e39959 100644 --- a/tensorflow/python/platform/flags_test.py +++ b/tensorflow/python/platform/flags_test.py @@ -17,11 +17,13 @@ from __future__ import absolute_import from __future__ import division from __future__ import print_function +import sys import unittest from absl import flags as absl_flags from tensorflow.python.platform import flags +from tensorflow.python.platform import test flags.DEFINE_string( @@ -48,8 +50,59 @@ flags.DEFINE_boolean( class FlagsTest(unittest.TestCase): - def test_global_flags_object(self): - self.assertIs(flags.FLAGS, absl_flags.FLAGS) + def setUp(self): + self.original_flags = flags.FlagValues() + self.wrapped_flags = flags._FlagValuesWrapper(self.original_flags) + flags.DEFINE_string( + 'test', 'default', 'test flag', flag_values=self.wrapped_flags) + + def test_attribute_overrides(self): + # Test that methods defined in absl.flags.FlagValues are the same as the + # wrapped ones. + self.assertEqual(flags.FLAGS.is_parsed, absl_flags.FLAGS.is_parsed) + + def test_getattr(self): + self.assertFalse(self.wrapped_flags.is_parsed()) + with test.mock.patch.object(sys, 'argv', new=['program', '--test=new']): + self.assertEqual('new', self.wrapped_flags.test) + self.assertTrue(self.wrapped_flags.is_parsed()) + + def test_setattr(self): + self.assertEqual('default', self.wrapped_flags.test) + self.wrapped_flags.test = 'new' + self.assertEqual('new', self.wrapped_flags.test) + + def test_delattr(self): + del self.wrapped_flags.test + self.assertNotIn('test', self.wrapped_flags) + with self.assertRaises(AttributeError): + _ = self.wrapped_flags.test + + def test_dir(self): + self.assertEqual(['test'], dir(self.wrapped_flags)) + + def test_getitem(self): + self.assertIs(self.original_flags['test'], self.wrapped_flags['test']) + + def test_setitem(self): + flag = flags.Flag(flags.ArgumentParser(), flags.ArgumentSerializer(), + 'fruit', 'apple', 'the fruit type') + self.wrapped_flags['fruit'] = flag + self.assertIs(self.original_flags['fruit'], self.wrapped_flags['fruit']) + self.assertEqual('apple', self.wrapped_flags.fruit) + + def test_len(self): + self.assertEqual(1, len(self.wrapped_flags)) + + def test_iter(self): + self.assertEqual(['test'], list(self.wrapped_flags)) + + def test_str(self): + self.assertEqual(str(self.wrapped_flags), str(self.original_flags)) + + def test_call(self): + self.wrapped_flags(['program', '--test=new']) + self.assertEqual('new', self.wrapped_flags.test) def test_keyword_arguments(self): test_cases = (