From a97c3c682a55787c053d1bda8de9e7c38d013d8f Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Fri, 18 Dec 2015 22:44:46 +0100 Subject: [PATCH] some optimisation --- test/test_option.py | 8 ---- tiramisu/option/baseoption.py | 68 ++++++++++++--------------- tiramisu/option/optiondescription.py | 7 +-- tiramisu/storage/dictionary/option.py | 47 +++++++++--------- 4 files changed, 60 insertions(+), 70 deletions(-) diff --git a/test/test_option.py b/test/test_option.py index e6406c1..bbe88a1 100644 --- a/test/test_option.py +++ b/test/test_option.py @@ -129,11 +129,3 @@ def test_option_multi(): raises(ValueError, "IntOption('test', '', multi=True, default_multi='yes')") #not default_multi with callback raises(ValueError, "IntOption('test', '', multi=True, default_multi=1, callback=a_func)") - - -def test_option_is_multi_by_default(): - assert IntOption('test', '').impl_is_empty_by_default() is True - assert IntOption('test', '', 1).impl_is_empty_by_default() is False - assert IntOption('test', '', multi=True).impl_is_empty_by_default() is True - assert IntOption('test', '', [1], multi=True).impl_is_empty_by_default() is False - assert IntOption('test', '', multi=True, default_multi=1).impl_is_empty_by_default() is True diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index f5c07b9..1805bde 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -32,21 +32,20 @@ from ..storage import get_storages_option StorageBase = get_storages_option('base') submulti = 2 -allowed_character = '[a-zA-Z\d\-_]' -name_regexp = re.compile(r'^[a-z]{0}*$'.format(allowed_character)) -forbidden_names = ('iter_all', 'iter_group', 'find', 'find_first', - 'make_dict', 'unwrap_from_path', 'read_only', - 'read_write', 'getowner', 'set_contexts') +name_regexp = re.compile(r'^[a-z][a-zA-Z\d\-_]*$') +forbidden_names = frozenset(['iter_all', 'iter_group', 'find', 'find_first', + 'make_dict', 'unwrap_from_path', 'read_only', + 'read_write', 'getowner', 'set_contexts']) def valid_name(name): "an option's name is a str and does not start with 'impl' or 'cfgimpl'" if not isinstance(name, str): # pragma: optional cover return False - if re.match(name_regexp, name) is not None and not name.startswith('_') \ - and name not in forbidden_names \ - and not name.startswith('impl_') \ - and not name.startswith('cfgimpl_'): + if re.match(name_regexp, name) is not None and \ + name not in forbidden_names and \ + not name.startswith('impl_') and \ + not name.startswith('cfgimpl_'): return True else: # pragma: optional cover return False @@ -140,20 +139,20 @@ class Base(StorageBase): allow_empty_list) if multi is not False and default is None: default = [] - self.impl_validate(default) - self._set_default_values(default, default_multi) - #callback is False in optiondescription + self.impl_validate(default, is_multi=_multi != 1) + self._set_default_values(default, default_multi, _multi != 1) + ##callback is False in optiondescription if callback is not False: - self.impl_set_callback(callback, callback_params) + self.impl_set_callback(callback, callback_params, _init=True) self.commit() - def impl_set_callback(self, callback, callback_params=None): + def impl_set_callback(self, callback, callback_params=None, _init=False): if callback is None and callback_params is not None: # pragma: optional cover - raise ValueError(_("params defined for a callback function but " - "no callback defined" - " yet for option {0}").format( - self.impl_getname())) - if self.impl_get_callback()[0] is not None: + raise ValueError(_("params defined for a callback function but " + "no callback defined" + " yet for option {0}").format( + self.impl_getname())) + if not _init and self.impl_get_callback()[0] is not None: raise ConfigError(_("a callback is already set for option {0}, " "cannot set another one's").format(self.impl_getname())) self._validate_callback(callback, callback_params) @@ -340,8 +339,8 @@ class BaseOption(Base): "frozen" (which has noting to do with the high level "freeze" propertie or "read_only" property) """ - if name not in ('_option', '_is_build_cache') \ - and not isinstance(value, tuple) and \ + if name not in ('_option', '_is_build_cache') and \ + not isinstance(value, tuple) and \ not name.startswith('_state') and \ not name == '_sa_instance_state': is_readonly = False @@ -479,7 +478,7 @@ class Option(OnlyOption): def impl_validate(self, value, context=undefined, validate=True, force_index=None, force_submulti_index=None, - current_opt=undefined): + current_opt=undefined, is_multi=None): """ :param value: the option's value :param context: Config's context @@ -577,7 +576,9 @@ class Option(OnlyOption): #if context is not undefined: # descr = context.cfgimpl_get_description() - if not self.impl_is_multi(): + if is_multi is None: + is_multi = self.impl_is_multi() + if not is_multi: do_validation(value, None, None) elif force_index is not None: if self.impl_is_submulti() and force_submulti_index is None: @@ -624,14 +625,6 @@ class Option(OnlyOption): def impl_get_master_slaves(self): return self._master_slaves - def impl_is_empty_by_default(self): - "no default value has been set yet" - if ((not self.impl_is_multi() and self.impl_getdefault() is None) or - (self.impl_is_multi() and (self.impl_getdefault() == [] - or None in self.impl_getdefault()))): - return True - return False - def impl_getdoc(self): "accesses the Option's doc" return self.impl_get_information('doc') @@ -791,10 +784,10 @@ class Option(OnlyOption): if callback is None: return default_multi = self.impl_getdefault_multi() - if (not self.impl_is_multi() and (self.impl_getdefault() is not None or - default_multi is not None)) or \ - (self.impl_is_multi() and (self.impl_getdefault() != [] or - default_multi is not None)): # pragma: optional cover + is_multi = self.impl_is_multi() + default = self.impl_getdefault() + if (not is_multi and (default is not None or default_multi is not None)) or \ + (is_multi and (default != [] or default_multi is not None)): # pragma: optional cover raise ValueError(_("default value not allowed if option: {0} " "is calculated").format(self.impl_getname())) @@ -980,8 +973,9 @@ class DynSymLinkOption(object): return base_path + '.' + self._dyn def impl_validate(self, value, context=undefined, validate=True, - force_index=None, force_submulti_index=None): + force_index=None, force_submulti_index=None, is_multi=None): return self._impl_getopt().impl_validate(value, context, validate, force_index, force_submulti_index, - current_opt=self) + current_opt=self, + is_multi=is_multi) diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index 46f1074..ecb7c86 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -24,7 +24,7 @@ import re from ..i18n import _ from ..setting import groups, undefined # , log -from .baseoption import BaseOption, SymLinkOption, allowed_character +from .baseoption import BaseOption, SymLinkOption from . import MasterSlaves from ..error import ConfigError, ConflictError from ..storage import get_storages_option @@ -32,7 +32,8 @@ from ..autolib import carry_out_calculation StorageOptionDescription = get_storages_option('optiondescription') -name_regexp = re.compile(r'^{0}*$'.format(allowed_character)) + +name_regexp = re.compile(r'^[a-zA-Z\d\-_]*$') class OptionDescription(BaseOption, StorageOptionDescription): @@ -81,7 +82,7 @@ class OptionDescription(BaseOption, StorageOptionDescription): def impl_getdoc(self): return self.impl_get_information('doc') - def impl_validate(self, *args): + def impl_validate(self, *args, **kwargs): """usefull for OptionDescription""" pass diff --git a/tiramisu/storage/dictionary/option.py b/tiramisu/storage/dictionary/option.py index d64bfac..16949ac 100644 --- a/tiramisu/storage/dictionary/option.py +++ b/tiramisu/storage/dictionary/option.py @@ -61,43 +61,45 @@ class StorageBase(object): def __init__(self, name, multi, warnings_only, doc, extra, calc_properties, requires, properties, allow_empty_list, opt=undefined): - self._name = name + _setattr = object.__setattr__ + _setattr(self, '_name', name) if doc is not undefined: - self._informations = {'doc': doc} + _setattr(self, '_informations', {'doc': doc}) if multi != 1: - self._multi = multi + _setattr(self, '_multi', multi) if extra is not None: - self._extra = extra + _setattr(self, '_extra', extra) if warnings_only is True: - self._warnings_only = warnings_only + _setattr(self, '_warnings_only', warnings_only) if calc_properties is not undefined: - self._calc_properties = calc_properties + _setattr(self, '_calc_properties', calc_properties) if requires is not undefined: - self._requires = requires + _setattr(self, '_requires', requires) if properties is not undefined: - self._properties = properties + _setattr(self, '_properties', properties) if opt is not undefined: - self._opt = opt + _setattr(self, '_opt', opt) if allow_empty_list is not undefined: - self._allow_empty_list = allow_empty_list + _setattr(self, '_allow_empty_list', allow_empty_list) - def _set_default_values(self, default, default_multi): - if ((self.impl_is_multi() and default != tuple()) or - (not self.impl_is_multi() and default is not None)): - if self.impl_is_multi(): + def _set_default_values(self, default, default_multi, is_multi): + _setattr = object.__setattr__ + if (is_multi and default != []) or \ + (not is_multi and default is not None): + if is_multi: default = tuple(default) - self._default = default + _setattr(self, '_default', default) - if self.impl_is_multi() and default_multi is not None: + if is_multi and default_multi is not None: try: self._validate(default_multi) except ValueError as err: # pragma: optional cover raise ValueError(_("invalid default_multi value {0} " "for option {1}: {2}").format( str(default_multi), - self.impl_getname(), err)) - self._default_multi = default_multi + self.impl_getname(), str(err))) + _setattr(self, '_default_multi', default_multi) # information def impl_set_information(self, key, value): @@ -302,8 +304,8 @@ class StorageBase(object): try: _multi = self._multi except AttributeError: - _multi = 1 - return _multi == 0 or _multi == 2 + return False + return _multi != 1 def impl_is_submulti(self): try: @@ -333,13 +335,14 @@ class StorageBase(object): def impl_getdefault(self): "accessing the default value" + is_multi = self.impl_is_multi() try: ret = self._default - if self.impl_is_multi(): + if is_multi: return list(ret) return ret except AttributeError: - if self.impl_is_multi(): + if is_multi: return [] return None