From e5d674ea5e77ddaa9dd0c74f24459c2ab319ccb5 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 21 Apr 2022 15:33:16 +0100 Subject: [PATCH] [configTools] accept either str or Option in Config mapping API --- Lib/fontTools/misc/configTools.py | 146 ++++++++++++++++++++---------- Tests/config_test.py | 47 +++++++--- 2 files changed, 130 insertions(+), 63 deletions(-) diff --git a/Lib/fontTools/misc/configTools.py b/Lib/fontTools/misc/configTools.py index 9a8ac7cf0..4a4531028 100644 --- a/Lib/fontTools/misc/configTools.py +++ b/Lib/fontTools/misc/configTools.py @@ -17,9 +17,11 @@ from typing import ( Callable, ClassVar, Dict, - Iterator, + Iterable, Mapping, MutableMapping, + Optional, + Set, Union, ) @@ -72,28 +74,29 @@ class ConfigValueValidationError(ConfigError): ) -_NO_VALUE = object() - - class ConfigUnknownOptionError(ConfigError): """Raised when a configuration option is unknown.""" - def __init__(self, name, value=_NO_VALUE): - super().__init__( - f"Config option {name} is unknown" - + ("" if value is _NO_VALUE else f" (with given value {repr(value)})") + def __init__(self, option_or_name): + name = ( + f"'{option_or_name.name}' (id={id(option_or_name)})>" + if isinstance(option_or_name, Option) + else f"'{option_or_name}'" ) + super().__init__(f"Config option {name} is unknown") -@dataclass +@dataclass(frozen=True) class Option: + name: str + """Unique name identifying the option (e.g. package.module:MY_OPTION).""" help: str """Help text for this option.""" default: Any """Default value for this option.""" parse: Callable[[str], Any] """Turn input (e.g. string) into proper type. Only when reading from file.""" - validate: Callable[[Any], bool] + validate: Optional[Callable[[Any], bool]] = None """Return true if the given value is an acceptable value.""" @@ -106,12 +109,14 @@ class Options(Mapping): """ __options: Dict[str, Option] + __cache: Set[Option] def __init__(self, other: "Options" = None) -> None: self.__options = {} + self.__cache = set() if other is not None: - for name, option in other.items(): - self.register_option(name, option) + for option in other.values(): + self.register_option(option) def register( self, @@ -119,18 +124,26 @@ class Options(Mapping): help: str, default: Any, parse: Callable[[str], Any], - validate: Callable[[Any], bool], + validate: Optional[Callable[[Any], bool]] = None, ) -> Option: - """Register a new option.""" - return self.register_option(name, Option(help, default, parse, validate)) + """Create and register a new option.""" + return self.register_option(Option(name, help, default, parse, validate)) - def register_option(self, name: str, option: Option) -> Option: + def register_option(self, option: Option) -> Option: """Register a new option.""" + name = option.name if name in self.__options: raise ConfigAlreadyRegisteredError(name) + # sanity check option values are unique + assert option not in self.__cache self.__options[name] = option + self.__cache.add(option) return option + def is_registered(self, option: Option) -> bool: + """Return True if the option object is already registered.""" + return option in self.__cache + def __getitem__(self, key: str) -> Option: return self.__options.__getitem__(key) @@ -157,7 +170,10 @@ _USE_GLOBAL_DEFAULT = object() class AbstractConfig(MutableMapping): """ Create a set of config values, optionally pre-filled with values from - the given dictionary. + the given dictionary or pre-existing config object. + + The class implements the MutableMapping protocol keyed by option name (`str`). + For convenience its methods accept either Option or str as the key parameter. .. seealso:: :meth:`set()` @@ -173,32 +189,68 @@ class AbstractConfig(MutableMapping): MyConfig.register_option( "test:option_name", "This is an option", 0, int, lambda v: isinstance(v, int)) cfg = MyConfig({"test:option_name": 10}) + """ options: ClassVar[Options] @classmethod - def register_option(cls, *args, **kwargs) -> Option: + def register_option( + cls, + name: str, + help: str, + default: Any, + parse: Callable[[str], Any], + validate: Optional[Callable[[Any], bool]] = None, + ) -> Option: """Register an available option in this config system.""" - return cls.options.register(*args, **kwargs) + return cls.options.register( + name, help=help, default=default, parse=parse, validate=validate + ) _values: Dict[str, Any] def __init__( self, - values: Union[AbstractConfig, Dict] = {}, - parse_values=False, - skip_unknown=False, + values: Union[AbstractConfig, Dict[Union[Option, str], Any]] = {}, + parse_values: bool = False, + skip_unknown: bool = False, ): self._values = {} values_dict = values._values if isinstance(values, AbstractConfig) else values for name, value in values_dict.items(): self.set(name, value, parse_values, skip_unknown) - def set(self, name: str, value: Any, parse_values=False, skip_unknown=False): + def _resolve_option(self, option_or_name: Union[Option, str]) -> Option: + if isinstance(option_or_name, Option): + option = option_or_name + if not self.options.is_registered(option): + raise ConfigUnknownOptionError(option) + return option + elif isinstance(option_or_name, str): + name = option_or_name + try: + return self.options[name] + except KeyError: + raise ConfigUnknownOptionError(name) + else: + raise TypeError( + "expected Option or str, found " + f"{type(option_or_name).__name__}: {option_or_name!r}" + ) + + def set( + self, + option_or_name: Union[Option, str], + value: Any, + parse_values: bool = False, + skip_unknown: bool = False, + ): """Set the value of an option. Args: + * `option_or_name`: an `Option` object or its name (`str`). + * `value`: the value to be assigned to given option. * `parse_values`: parse the configuration value from a string into its proper type, as per its `Option` object. The default behavior is to raise `ConfigValueValidationError` when the value @@ -210,15 +262,12 @@ class AbstractConfig(MutableMapping): (e.g. for a later version of fontTools) """ try: - option = self.options[name] - except KeyError: + option = self._resolve_option(option_or_name) + except ConfigUnknownOptionError as e: if skip_unknown: - log.debug( - "Config option %s is unknown (with given value %r)", name, value - ) + log.debug(str(e)) return - else: - raise ConfigUnknownOptionError(name, value) + raise # Can be useful if the values come from a source that doesn't have # strict typing (.ini file? Terminal input?) @@ -226,14 +275,16 @@ class AbstractConfig(MutableMapping): try: value = option.parse(value) except Exception as e: - raise ConfigValueParsingError(name, value) from e + raise ConfigValueParsingError(option.name, value) from e - if not option.validate(value): - raise ConfigValueValidationError(name, value) + if option.validate is not None and not option.validate(value): + raise ConfigValueValidationError(option.name, value) - self._values[name] = value + self._values[option.name] = value - def get(self, name: str, default=_USE_GLOBAL_DEFAULT): + def get( + self, option_or_name: Union[Option, str], default: Any = _USE_GLOBAL_DEFAULT + ) -> Any: """ Get the value of an option. The value which is returned is the first provided among: @@ -256,28 +307,27 @@ class AbstractConfig(MutableMapping): still pass the option to the function call, but will favour the new config mechanism if the given font specifies a value for that option. """ - if name in self._values: - return self._values[name] + option = self._resolve_option(option_or_name) + if option.name in self._values: + return self._values[option.name] if default is not _USE_GLOBAL_DEFAULT: return default - try: - return self.options[name].default - except KeyError as e: - raise ConfigUnknownOptionError(name) from e + return option.default def copy(self): return self.__class__(self._values) - def __getitem__(self, name: str) -> Any: - return self.get(name) + def __getitem__(self, option_or_name: Union[Option, str]) -> Any: + return self.get(option_or_name) - def __setitem__(self, name: str, value: Any) -> None: - return self.set(name, value) + def __setitem__(self, option_or_name: Union[Option, str], value: Any) -> None: + return self.set(option_or_name, value) - def __delitem__(self, name: str) -> None: - del self._values[name] + def __delitem__(self, option_or_name: Union[Option, str]) -> None: + option = self._resolve_option(option_or_name) + del self._values[option.name] - def __iter__(self) -> Iterator: + def __iter__(self) -> Iterable[str]: return self._values.__iter__() def __len__(self) -> int: diff --git a/Tests/config_test.py b/Tests/config_test.py index 34ecbc6cd..58163913e 100644 --- a/Tests/config_test.py +++ b/Tests/config_test.py @@ -1,6 +1,7 @@ from fontTools.misc.configTools import AbstractConfig, Options import pytest from fontTools.config import ( + OPTIONS, Config, ConfigUnknownOptionError, ConfigValueParsingError, @@ -18,6 +19,7 @@ def test_can_register_option(): validate=lambda v: v == True or v == False, ) + assert MY_OPTION.name == "tests:MY_OPTION" assert ( MY_OPTION.help == "Test option, value should be True or False, default = True" ) @@ -27,36 +29,51 @@ def test_can_register_option(): ttFont = TTFont(cfg={"tests:MY_OPTION": True}) assert True == ttFont.cfg.get("tests:MY_OPTION") + assert True == ttFont.cfg.get(MY_OPTION) -COMPRESSION_LEVEL = "fontTools.otlLib.optimize.gpos:COMPRESSION_LEVEL" +# to parametrize tests of Config mapping interface accepting either a str or Option +@pytest.fixture( + params=[ + pytest.param("fontTools.otlLib.optimize.gpos:COMPRESSION_LEVEL", id="str"), + pytest.param( + OPTIONS["fontTools.otlLib.optimize.gpos:COMPRESSION_LEVEL"], id="Option" + ), + ] +) +def COMPRESSION_LEVEL(request): + return request.param -def test_ttfont_has_config(): +def test_ttfont_has_config(COMPRESSION_LEVEL): ttFont = TTFont(cfg={COMPRESSION_LEVEL: 8}) assert 8 == ttFont.cfg.get(COMPRESSION_LEVEL) -def test_ttfont_can_take_superset_of_fonttools_config(): +def test_ttfont_can_take_superset_of_fonttools_config(COMPRESSION_LEVEL): # Create MyConfig with all options from fontTools.config plus some my_options = Options(Config.options) - my_options.register("custom:my_option", "help", "default", str, any) + MY_OPTION = my_options.register("custom:my_option", "help", "default", str, any) class MyConfig(AbstractConfig): options = my_options ttFont = TTFont(cfg=MyConfig({"custom:my_option": "my_value"})) assert 0 == ttFont.cfg.get(COMPRESSION_LEVEL) - assert "my_value" == ttFont.cfg.get("custom:my_option") + assert "my_value" == ttFont.cfg.get(MY_OPTION) + + # but the default Config doens't know about MY_OPTION + with pytest.raises(ConfigUnknownOptionError): + TTFont(cfg={MY_OPTION: "my_value"}) -def test_no_config_returns_default_values(): +def test_no_config_returns_default_values(COMPRESSION_LEVEL): ttFont = TTFont() assert 0 == ttFont.cfg.get(COMPRESSION_LEVEL) assert 3 == ttFont.cfg.get(COMPRESSION_LEVEL, 3) -def test_can_set_config(): +def test_can_set_config(COMPRESSION_LEVEL): ttFont = TTFont() ttFont.cfg.set(COMPRESSION_LEVEL, 5) assert 5 == ttFont.cfg.get(COMPRESSION_LEVEL) @@ -64,7 +81,7 @@ def test_can_set_config(): assert 6 == ttFont.cfg.get(COMPRESSION_LEVEL) -def test_different_ttfonts_have_different_configs(): +def test_different_ttfonts_have_different_configs(COMPRESSION_LEVEL): cfg = Config({COMPRESSION_LEVEL: 5}) ttFont1 = TTFont(cfg=cfg) ttFont2 = TTFont(cfg=cfg) @@ -78,19 +95,19 @@ def test_cannot_set_inexistent_key(): TTFont(cfg={"notALib.notAModule.inexistent": 4}) -def test_value_not_parsed_by_default(): +def test_value_not_parsed_by_default(COMPRESSION_LEVEL): # Note: value given as a string with pytest.raises(ConfigValueValidationError): TTFont(cfg={COMPRESSION_LEVEL: "8"}) -def test_value_gets_parsed_if_asked(): +def test_value_gets_parsed_if_asked(COMPRESSION_LEVEL): # Note: value given as a string ttFont = TTFont(cfg=Config({COMPRESSION_LEVEL: "8"}, parse_values=True)) assert 8 == ttFont.cfg.get(COMPRESSION_LEVEL) -def test_value_parsing_can_error(): +def test_value_parsing_can_error(COMPRESSION_LEVEL): with pytest.raises(ConfigValueParsingError): TTFont( cfg=Config( @@ -100,19 +117,19 @@ def test_value_parsing_can_error(): ) -def test_value_gets_validated(): +def test_value_gets_validated(COMPRESSION_LEVEL): # Note: 12 is not a valid value for GPOS compression level (must be in 0-9) with pytest.raises(ConfigValueValidationError): TTFont(cfg={COMPRESSION_LEVEL: 12}) -def test_implements_mutable_mapping(): +def test_implements_mutable_mapping(COMPRESSION_LEVEL): cfg = Config() cfg[COMPRESSION_LEVEL] = 2 assert 2 == cfg[COMPRESSION_LEVEL] - assert [COMPRESSION_LEVEL] == list(iter(cfg)) + assert list(iter(cfg)) assert 1 == len(cfg) del cfg[COMPRESSION_LEVEL] assert 0 == cfg[COMPRESSION_LEVEL] - assert [] == list(iter(cfg)) + assert not list(iter(cfg)) assert 0 == len(cfg)