From be755cb76df0aa7f11bb34c3712d9913a89c45fd Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Mon, 7 Nov 2022 08:02:49 -0800 Subject: [PATCH 01/10] Fix DEFINE_multi_enum_class's additional kwargs (e.g. short_name). They were passed to the wrong function. PiperOrigin-RevId: 486662821 Change-Id: I9015cad60073165eeca379bf2419612c33bcae0e --- CHANGELOG.md | 5 +++++ absl/flags/_defines.py | 10 ++++++++-- absl/flags/tests/flags_test.py | 11 +++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae82a55d..5203fdd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com). Nothing notable unreleased. +### Fixed + +* (flags) Additional kwargs (e.g. `short_name=`) to `DEFINE_multi_enum_class` + are now correctly passed to the underlying `Flag` object. + ## 1.3.0 (2022-10-11) ### Added diff --git a/absl/flags/_defines.py b/absl/flags/_defines.py index dce53ea2..61354e94 100644 --- a/absl/flags/_defines.py +++ b/absl/flags/_defines.py @@ -859,11 +859,17 @@ def DEFINE_multi_enum_class( # pylint: disable=invalid-name,redefined-builtin """ return DEFINE_flag( _flag.MultiEnumClassFlag( - name, default, help, enum_class, case_sensitive=case_sensitive), + name, + default, + help, + enum_class, + case_sensitive=case_sensitive, + **args, + ), flag_values, module_name, required=required, - **args) + ) def DEFINE_alias( # pylint: disable=invalid-name diff --git a/absl/flags/tests/flags_test.py b/absl/flags/tests/flags_test.py index 77ed307e..7cacbc84 100644 --- a/absl/flags/tests/flags_test.py +++ b/absl/flags/tests/flags_test.py @@ -1591,6 +1591,17 @@ def test_bad_multi_enum_flags(self): class MultiEnumClassFlagsTest(absltest.TestCase): + def test_short_name(self): + fv = flags.FlagValues() + flags.DEFINE_multi_enum_class( + 'fruit', + None, + Fruit, + 'Enum option that can occur multiple times', + flag_values=fv, + short_name='me') + self.assertEqual(fv['fruit'].short_name, 'me') + def test_define_results_in_registered_flag_with_none(self): fv = flags.FlagValues() enum_defaults = None From 0947bd875717674dfc23aa580d861b8e6f04c758 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 9 Nov 2022 14:24:16 -0800 Subject: [PATCH 02/10] Export `set_default` in `flags/__init__.pyi`. This method is defined in `_defines.pyi`, but since it's not exported in `__init__.pyi`, type checkers will complain when users call `flags.set_default(...)`. PiperOrigin-RevId: 487346934 Change-Id: Idf7cffc37cea1c98362d5ea6242b4e85089f13c9 --- absl/flags/__init__.pyi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/absl/flags/__init__.pyi b/absl/flags/__init__.pyi index 4eee59e2..7bf6842e 100644 --- a/absl/flags/__init__.pyi +++ b/absl/flags/__init__.pyi @@ -52,6 +52,9 @@ mark_flags_as_required = _validators.mark_flags_as_required mark_flags_as_mutual_exclusive = _validators.mark_flags_as_mutual_exclusive mark_bool_flags_as_mutual_exclusive = _validators.mark_bool_flags_as_mutual_exclusive +# Flag modifiers. +set_default = _defines.set_default + # Key flag related functions. declare_key_flag = _defines.declare_key_flag adopt_module_key_flags = _defines.adopt_module_key_flags From 7a8350ca9bae7000420b689195e3c48a390ca88b Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 29 Nov 2022 08:03:27 -0800 Subject: [PATCH 03/10] Improve error message when assigning a flag. PiperOrigin-RevId: 491637962 Change-Id: I324ce6cb77beb5433cb7f7b3773bf50408040c63 --- absl/flags/_flagvalues.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/absl/flags/_flagvalues.py b/absl/flags/_flagvalues.py index 937dc6c2..6661b783 100644 --- a/absl/flags/_flagvalues.py +++ b/absl/flags/_flagvalues.py @@ -411,7 +411,9 @@ def __setitem__(self, name, flag): """Registers a new flag variable.""" fl = self._flags() if not isinstance(flag, _flag.Flag): - raise _exceptions.IllegalFlagValueError(flag) + raise _exceptions.IllegalFlagValueError( + f'Expect Flag instances, found type {type(flag)}. ' + "Maybe you didn't mean to use FlagValue.__setitem__?") if not isinstance(name, str): raise _exceptions.Error('Flag name must be a string') if not name: From 490830bb963bd436a87ba81e4b8decb42c685d2c Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 29 Nov 2022 08:40:03 -0800 Subject: [PATCH 04/10] Add type hinting to the flagsaver module. PiperOrigin-RevId: 491646527 Change-Id: I4e0472cf563eef00bb14d9843f60c3ac6052e3b0 --- absl/flags/_flag.pyi | 3 ++- absl/testing/flagsaver.py | 50 ++++++++++++++++++++++++++------------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/absl/flags/_flag.pyi b/absl/flags/_flag.pyi index 9b4a3d3a..35066445 100644 --- a/absl/flags/_flag.pyi +++ b/absl/flags/_flag.pyi @@ -20,7 +20,7 @@ import functools from absl.flags import _argument_parser import enum -from typing import Text, TypeVar, Generic, Iterable, Type, List, Optional, Any, Union, Sequence +from typing import Callable, Text, TypeVar, Generic, Iterable, Type, List, Optional, Any, Union, Sequence _T = TypeVar('_T') _ET = TypeVar('_ET', bound=enum.Enum) @@ -44,6 +44,7 @@ class Flag(Generic[_T]): using_default_value = ... # type: bool allow_overwrite = ... # type: bool allow_using_method_names = ... # type: bool + validators = ... # type: List[Callable[[Any], bool]] def __init__(self, parser: _argument_parser.ArgumentParser[_T], diff --git a/absl/testing/flagsaver.py b/absl/testing/flagsaver.py index 37926d7a..774c698c 100644 --- a/absl/testing/flagsaver.py +++ b/absl/testing/flagsaver.py @@ -11,7 +11,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - """Decorator and context manager for saving and restoring flag values. There are many ways to save and restore. Always use the most convenient method @@ -61,11 +60,26 @@ def some_func(): import functools import inspect +from typing import overload, Any, Callable, Mapping, Tuple, TypeVar from absl import flags FLAGS = flags.FLAGS +# The type of pre/post wrapped functions. +_CallableT = TypeVar('_CallableT', bound=Callable) + + +@overload +def flagsaver(*args: Tuple[flags.FlagHolder, Any], + **kwargs: Any) -> '_FlagOverrider': + ... + + +@overload +def flagsaver(func: _CallableT) -> _CallableT: + ... + def flagsaver(*args, **kwargs): """The main flagsaver interface. See module doc for usage.""" @@ -94,12 +108,14 @@ def flagsaver(*args, **kwargs): return _FlagOverrider(**kwargs) -def save_flag_values(flag_values=FLAGS): +def save_flag_values( + flag_values: flags.FlagValues = FLAGS) -> Mapping[str, Mapping[str, Any]]: """Returns copy of flag values as a dict. Args: - flag_values: FlagValues, the FlagValues instance with which the flag will - be saved. This should almost never need to be overridden. + flag_values: FlagValues, the FlagValues instance with which the flag will be + saved. This should almost never need to be overridden. + Returns: Dictionary mapping keys to values. Keys are flag names, values are corresponding ``__dict__`` members. E.g. ``{'key': value_dict, ...}``. @@ -107,13 +123,14 @@ def save_flag_values(flag_values=FLAGS): return {name: _copy_flag_dict(flag_values[name]) for name in flag_values} -def restore_flag_values(saved_flag_values, flag_values=FLAGS): +def restore_flag_values(saved_flag_values: Mapping[str, Mapping[str, Any]], + flag_values: flags.FlagValues = FLAGS): """Restores flag values based on the dictionary of flag values. Args: saved_flag_values: {'flag_name': value_dict, ...} - flag_values: FlagValues, the FlagValues instance from which the flag will - be restored. This should almost never need to be overridden. + flag_values: FlagValues, the FlagValues instance from which the flag will be + restored. This should almost never need to be overridden. """ new_flag_names = list(flag_values) for name in new_flag_names: @@ -127,23 +144,24 @@ def restore_flag_values(saved_flag_values, flag_values=FLAGS): flag_values[name].__dict__ = saved -def _wrap(func, overrides): +def _wrap(func: _CallableT, overrides: Mapping[str, Any]) -> _CallableT: """Creates a wrapper function that saves/restores flag values. Args: - func: function object - This will be called between saving flags and - restoring flags. - overrides: {str: object} - Flag names mapped to their values. These flags - will be set after saving the original flag state. + func: This will be called between saving flags and restoring flags. + overrides: Flag names mapped to their values. These flags will be set after + saving the original flag state. Returns: - return value from func() + A wrapped version of func. """ + @functools.wraps(func) def _flagsaver_wrapper(*args, **kwargs): """Wrapper function that saves and restores flags.""" with _FlagOverrider(**overrides): return func(*args, **kwargs) + return _flagsaver_wrapper @@ -154,11 +172,11 @@ class _FlagOverrider(object): completes. """ - def __init__(self, **overrides): + def __init__(self, **overrides: Any): self._overrides = overrides self._saved_flag_values = None - def __call__(self, func): + def __call__(self, func: _CallableT) -> _CallableT: if inspect.isclass(func): raise TypeError('flagsaver cannot be applied to a class.') return _wrap(func, self._overrides) @@ -176,7 +194,7 @@ def __exit__(self, exc_type, exc_value, traceback): restore_flag_values(self._saved_flag_values, FLAGS) -def _copy_flag_dict(flag): +def _copy_flag_dict(flag: flags.Flag) -> Mapping[str, Any]: """Returns a copy of the flag object's ``__dict__``. It's mostly a shallow copy of the ``__dict__``, except it also does a shallow From 6c83adcf2c35d1e41dee63c3199189b546731b6d Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 30 Nov 2022 07:41:01 -0800 Subject: [PATCH 05/10] Add type hinting to the flagsaver module. PiperOrigin-RevId: 491913342 Change-Id: Ib282912288cbef3f6788e9be18cb82360c238773 --- absl/flags/_flag.pyi | 3 +-- absl/testing/flagsaver.py | 50 +++++++++++++-------------------------- 2 files changed, 17 insertions(+), 36 deletions(-) diff --git a/absl/flags/_flag.pyi b/absl/flags/_flag.pyi index 35066445..9b4a3d3a 100644 --- a/absl/flags/_flag.pyi +++ b/absl/flags/_flag.pyi @@ -20,7 +20,7 @@ import functools from absl.flags import _argument_parser import enum -from typing import Callable, Text, TypeVar, Generic, Iterable, Type, List, Optional, Any, Union, Sequence +from typing import Text, TypeVar, Generic, Iterable, Type, List, Optional, Any, Union, Sequence _T = TypeVar('_T') _ET = TypeVar('_ET', bound=enum.Enum) @@ -44,7 +44,6 @@ class Flag(Generic[_T]): using_default_value = ... # type: bool allow_overwrite = ... # type: bool allow_using_method_names = ... # type: bool - validators = ... # type: List[Callable[[Any], bool]] def __init__(self, parser: _argument_parser.ArgumentParser[_T], diff --git a/absl/testing/flagsaver.py b/absl/testing/flagsaver.py index 774c698c..37926d7a 100644 --- a/absl/testing/flagsaver.py +++ b/absl/testing/flagsaver.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + """Decorator and context manager for saving and restoring flag values. There are many ways to save and restore. Always use the most convenient method @@ -60,26 +61,11 @@ def some_func(): import functools import inspect -from typing import overload, Any, Callable, Mapping, Tuple, TypeVar from absl import flags FLAGS = flags.FLAGS -# The type of pre/post wrapped functions. -_CallableT = TypeVar('_CallableT', bound=Callable) - - -@overload -def flagsaver(*args: Tuple[flags.FlagHolder, Any], - **kwargs: Any) -> '_FlagOverrider': - ... - - -@overload -def flagsaver(func: _CallableT) -> _CallableT: - ... - def flagsaver(*args, **kwargs): """The main flagsaver interface. See module doc for usage.""" @@ -108,14 +94,12 @@ def flagsaver(*args, **kwargs): return _FlagOverrider(**kwargs) -def save_flag_values( - flag_values: flags.FlagValues = FLAGS) -> Mapping[str, Mapping[str, Any]]: +def save_flag_values(flag_values=FLAGS): """Returns copy of flag values as a dict. Args: - flag_values: FlagValues, the FlagValues instance with which the flag will be - saved. This should almost never need to be overridden. - + flag_values: FlagValues, the FlagValues instance with which the flag will + be saved. This should almost never need to be overridden. Returns: Dictionary mapping keys to values. Keys are flag names, values are corresponding ``__dict__`` members. E.g. ``{'key': value_dict, ...}``. @@ -123,14 +107,13 @@ def save_flag_values( return {name: _copy_flag_dict(flag_values[name]) for name in flag_values} -def restore_flag_values(saved_flag_values: Mapping[str, Mapping[str, Any]], - flag_values: flags.FlagValues = FLAGS): +def restore_flag_values(saved_flag_values, flag_values=FLAGS): """Restores flag values based on the dictionary of flag values. Args: saved_flag_values: {'flag_name': value_dict, ...} - flag_values: FlagValues, the FlagValues instance from which the flag will be - restored. This should almost never need to be overridden. + flag_values: FlagValues, the FlagValues instance from which the flag will + be restored. This should almost never need to be overridden. """ new_flag_names = list(flag_values) for name in new_flag_names: @@ -144,24 +127,23 @@ def restore_flag_values(saved_flag_values: Mapping[str, Mapping[str, Any]], flag_values[name].__dict__ = saved -def _wrap(func: _CallableT, overrides: Mapping[str, Any]) -> _CallableT: +def _wrap(func, overrides): """Creates a wrapper function that saves/restores flag values. Args: - func: This will be called between saving flags and restoring flags. - overrides: Flag names mapped to their values. These flags will be set after - saving the original flag state. + func: function object - This will be called between saving flags and + restoring flags. + overrides: {str: object} - Flag names mapped to their values. These flags + will be set after saving the original flag state. Returns: - A wrapped version of func. + return value from func() """ - @functools.wraps(func) def _flagsaver_wrapper(*args, **kwargs): """Wrapper function that saves and restores flags.""" with _FlagOverrider(**overrides): return func(*args, **kwargs) - return _flagsaver_wrapper @@ -172,11 +154,11 @@ class _FlagOverrider(object): completes. """ - def __init__(self, **overrides: Any): + def __init__(self, **overrides): self._overrides = overrides self._saved_flag_values = None - def __call__(self, func: _CallableT) -> _CallableT: + def __call__(self, func): if inspect.isclass(func): raise TypeError('flagsaver cannot be applied to a class.') return _wrap(func, self._overrides) @@ -194,7 +176,7 @@ def __exit__(self, exc_type, exc_value, traceback): restore_flag_values(self._saved_flag_values, FLAGS) -def _copy_flag_dict(flag: flags.Flag) -> Mapping[str, Any]: +def _copy_flag_dict(flag): """Returns a copy of the flag object's ``__dict__``. It's mostly a shallow copy of the ``__dict__``, except it also does a shallow From c4ab3bf1a95ce0ffa550984fc1cdfe9ccd777ed4 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 30 Nov 2022 08:30:02 -0800 Subject: [PATCH 06/10] Add a pytype disable for a type error. PiperOrigin-RevId: 491925237 Change-Id: I294f9993b4f1944e1d28ba54a5b14ae910b2714f --- absl/testing/absltest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/testing/absltest.py b/absl/testing/absltest.py index 9071f8f6..e14cfcb8 100644 --- a/absl/testing/absltest.py +++ b/absl/testing/absltest.py @@ -525,7 +525,7 @@ def open_bytes(self, mode='rb'): 'file in binary mode'.format(mode)) if 'b' not in mode: mode += 'b' - cm = self._open(mode, encoding=None, errors=None) + cm = self._open(mode, encoding=None, errors=None) # pytype: disable=wrong-arg-types return cm # TODO(b/123775699): Once pytype supports typing.Literal, use overload and From 7ab6c0a9e8b9866fff6c84b9d4cbe8eef5f73b73 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Wed, 30 Nov 2022 12:17:33 -0800 Subject: [PATCH 07/10] Fix the annotation of the `_open` method, and remove the previous pytype disable. PiperOrigin-RevId: 491986082 Change-Id: If45cf848ebbd200399bfa2bcda9b54a7ae4ef0e8 --- absl/testing/absltest.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/absl/testing/absltest.py b/absl/testing/absltest.py index e14cfcb8..ead42809 100644 --- a/absl/testing/absltest.py +++ b/absl/testing/absltest.py @@ -525,7 +525,7 @@ def open_bytes(self, mode='rb'): 'file in binary mode'.format(mode)) if 'b' not in mode: mode += 'b' - cm = self._open(mode, encoding=None, errors=None) # pytype: disable=wrong-arg-types + cm = self._open(mode, encoding=None, errors=None) return cm # TODO(b/123775699): Once pytype supports typing.Literal, use overload and @@ -533,7 +533,10 @@ def open_bytes(self, mode='rb'): # currently `Any` to avoid [bad-return-type] errors in the open_* methods. @contextlib.contextmanager def _open( - self, mode: str, encoding: str = 'utf8', errors: str = 'strict' + self, + mode: str, + encoding: Optional[str] = 'utf8', + errors: Optional[str] = 'strict', ) -> Iterator[Any]: with io.open( self.full_path, mode=mode, encoding=encoding, errors=errors) as fp: From 916113a2ec897568959dbad330ec77939059fe27 Mon Sep 17 00:00:00 2001 From: Nam Nguyen Date: Wed, 30 Nov 2022 16:44:19 -0800 Subject: [PATCH 08/10] Fix trivial typo: create_tempdir --> create_tempfile. PiperOrigin-RevId: 492053477 Change-Id: I40571b637eadac394b6f85dd05bc26932a9e7126 --- absl/testing/absltest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/testing/absltest.py b/absl/testing/absltest.py index ead42809..1bbcee74 100644 --- a/absl/testing/absltest.py +++ b/absl/testing/absltest.py @@ -641,7 +641,7 @@ def test_foo(self): self.assertTrue(os.path.exists(expected_paths[1])) self.assertEqual('foo', out_log.read_text()) - See also: :meth:`create_tempdir` for creating temporary files. + See also: :meth:`create_tempfile` for creating temporary files. Args: name: Optional name of the directory. If not given, a unique From b568abfd3be3480f451aec4923ada17d3acc6734 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 2 Dec 2022 12:02:17 -0800 Subject: [PATCH 09/10] Add type hinting to the flagsaver module. PiperOrigin-RevId: 492525537 Change-Id: Id30f2c466c3ea798f9207346d38bdfd754521c08 --- absl/flags/_flag.pyi | 3 ++- absl/testing/flagsaver.py | 50 ++++++++++++++++++++++++++------------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/absl/flags/_flag.pyi b/absl/flags/_flag.pyi index 9b4a3d3a..35066445 100644 --- a/absl/flags/_flag.pyi +++ b/absl/flags/_flag.pyi @@ -20,7 +20,7 @@ import functools from absl.flags import _argument_parser import enum -from typing import Text, TypeVar, Generic, Iterable, Type, List, Optional, Any, Union, Sequence +from typing import Callable, Text, TypeVar, Generic, Iterable, Type, List, Optional, Any, Union, Sequence _T = TypeVar('_T') _ET = TypeVar('_ET', bound=enum.Enum) @@ -44,6 +44,7 @@ class Flag(Generic[_T]): using_default_value = ... # type: bool allow_overwrite = ... # type: bool allow_using_method_names = ... # type: bool + validators = ... # type: List[Callable[[Any], bool]] def __init__(self, parser: _argument_parser.ArgumentParser[_T], diff --git a/absl/testing/flagsaver.py b/absl/testing/flagsaver.py index 37926d7a..774c698c 100644 --- a/absl/testing/flagsaver.py +++ b/absl/testing/flagsaver.py @@ -11,7 +11,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - """Decorator and context manager for saving and restoring flag values. There are many ways to save and restore. Always use the most convenient method @@ -61,11 +60,26 @@ def some_func(): import functools import inspect +from typing import overload, Any, Callable, Mapping, Tuple, TypeVar from absl import flags FLAGS = flags.FLAGS +# The type of pre/post wrapped functions. +_CallableT = TypeVar('_CallableT', bound=Callable) + + +@overload +def flagsaver(*args: Tuple[flags.FlagHolder, Any], + **kwargs: Any) -> '_FlagOverrider': + ... + + +@overload +def flagsaver(func: _CallableT) -> _CallableT: + ... + def flagsaver(*args, **kwargs): """The main flagsaver interface. See module doc for usage.""" @@ -94,12 +108,14 @@ def flagsaver(*args, **kwargs): return _FlagOverrider(**kwargs) -def save_flag_values(flag_values=FLAGS): +def save_flag_values( + flag_values: flags.FlagValues = FLAGS) -> Mapping[str, Mapping[str, Any]]: """Returns copy of flag values as a dict. Args: - flag_values: FlagValues, the FlagValues instance with which the flag will - be saved. This should almost never need to be overridden. + flag_values: FlagValues, the FlagValues instance with which the flag will be + saved. This should almost never need to be overridden. + Returns: Dictionary mapping keys to values. Keys are flag names, values are corresponding ``__dict__`` members. E.g. ``{'key': value_dict, ...}``. @@ -107,13 +123,14 @@ def save_flag_values(flag_values=FLAGS): return {name: _copy_flag_dict(flag_values[name]) for name in flag_values} -def restore_flag_values(saved_flag_values, flag_values=FLAGS): +def restore_flag_values(saved_flag_values: Mapping[str, Mapping[str, Any]], + flag_values: flags.FlagValues = FLAGS): """Restores flag values based on the dictionary of flag values. Args: saved_flag_values: {'flag_name': value_dict, ...} - flag_values: FlagValues, the FlagValues instance from which the flag will - be restored. This should almost never need to be overridden. + flag_values: FlagValues, the FlagValues instance from which the flag will be + restored. This should almost never need to be overridden. """ new_flag_names = list(flag_values) for name in new_flag_names: @@ -127,23 +144,24 @@ def restore_flag_values(saved_flag_values, flag_values=FLAGS): flag_values[name].__dict__ = saved -def _wrap(func, overrides): +def _wrap(func: _CallableT, overrides: Mapping[str, Any]) -> _CallableT: """Creates a wrapper function that saves/restores flag values. Args: - func: function object - This will be called between saving flags and - restoring flags. - overrides: {str: object} - Flag names mapped to their values. These flags - will be set after saving the original flag state. + func: This will be called between saving flags and restoring flags. + overrides: Flag names mapped to their values. These flags will be set after + saving the original flag state. Returns: - return value from func() + A wrapped version of func. """ + @functools.wraps(func) def _flagsaver_wrapper(*args, **kwargs): """Wrapper function that saves and restores flags.""" with _FlagOverrider(**overrides): return func(*args, **kwargs) + return _flagsaver_wrapper @@ -154,11 +172,11 @@ class _FlagOverrider(object): completes. """ - def __init__(self, **overrides): + def __init__(self, **overrides: Any): self._overrides = overrides self._saved_flag_values = None - def __call__(self, func): + def __call__(self, func: _CallableT) -> _CallableT: if inspect.isclass(func): raise TypeError('flagsaver cannot be applied to a class.') return _wrap(func, self._overrides) @@ -176,7 +194,7 @@ def __exit__(self, exc_type, exc_value, traceback): restore_flag_values(self._saved_flag_values, FLAGS) -def _copy_flag_dict(flag): +def _copy_flag_dict(flag: flags.Flag) -> Mapping[str, Any]: """Returns a copy of the flag object's ``__dict__``. It's mostly a shallow copy of the ``__dict__``, except it also does a shallow From 814e1f373cd83041cf34b6916586a5ed1c1253ce Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Mon, 12 Dec 2022 20:14:32 -0800 Subject: [PATCH 10/10] Merge changes from github. PR: https://github.com/abseil/abseil-py/pull/204 It still needs `dirs` since we have an internal patch that uses a list of dirs for our internal machines. PiperOrigin-RevId: 494901390 Change-Id: Ib3a549d71545f91d02b1282afb11708c65fb8348 --- CHANGELOG.md | 5 ++++- absl/logging/__init__.py | 6 +++++- absl/logging/tests/logging_test.py | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5203fdd6..56f832b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com). ## Unreleased -Nothing notable unreleased. +### Changed + +* (logging) If no log dir is specified `logging.find_log_dir()` now falls back + to `tempfile.gettempdir()` instead of `/tmp/`. ### Fixed diff --git a/absl/logging/__init__.py b/absl/logging/__init__.py index c0ba4b0f..961b3dcf 100644 --- a/absl/logging/__init__.py +++ b/absl/logging/__init__.py @@ -86,6 +86,7 @@ import socket import struct import sys +import tempfile import threading import time import timeit @@ -707,6 +708,9 @@ def find_log_dir(log_dir=None): OSError: raised in Python 2 when it cannot find a log directory. """ # Get a list of possible log dirs (will try to use them in order). + # NOTE: Google's internal implementation has a special handling for Google + # machines, which uses a list of directories. Hence the following uses `dirs` + # instead of a single directory. if log_dir: # log_dir was explicitly specified as an arg, so use it and it alone. dirs = [log_dir] @@ -715,7 +719,7 @@ def find_log_dir(log_dir=None): # behavior of the same flag in logging.cc). dirs = [FLAGS['log_dir'].value] else: - dirs = ['/tmp/', './'] + dirs = [tempfile.gettempdir()] # Find the first usable log dir. for d in dirs: diff --git a/absl/logging/tests/logging_test.py b/absl/logging/tests/logging_test.py index e5c4fccb..1c337f9a 100644 --- a/absl/logging/tests/logging_test.py +++ b/absl/logging/tests/logging_test.py @@ -706,7 +706,7 @@ def test_find_log_dir_with_hda_tmp(self): os.path.isdir.return_value = True os.access.return_value = True log_dir = logging.find_log_dir() - self.assertEqual('/tmp/', log_dir) + self.assertEqual(tempfile.gettempdir(), log_dir) @flagsaver.flagsaver(log_dir='') def test_find_log_dir_with_tmp(self): @@ -714,10 +714,10 @@ def test_find_log_dir_with_tmp(self): mock.patch.object(os.path, 'exists'), \ mock.patch.object(os.path, 'isdir'): os.path.exists.return_value = False - os.path.isdir.side_effect = lambda path: path == '/tmp/' + os.path.isdir.side_effect = lambda path: path == tempfile.gettempdir() os.access.return_value = True log_dir = logging.find_log_dir() - self.assertEqual('/tmp/', log_dir) + self.assertEqual(tempfile.gettempdir(), log_dir) def test_find_log_dir_with_nothing(self): with mock.patch.object(os.path, 'exists'), \