From 19b676967d4da9c2b0a6a152031e606998e317dd Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Wed, 14 Sep 2016 20:17:25 +0200 Subject: [PATCH] better propertyerror message --- test/test_option_setting.py | 64 ++++++++++++++++++++++++++++++++++ tiramisu/error.py | 32 ++++++++++++++++- tiramisu/option/baseoption.py | 32 ++++++++--------- tiramisu/option/masterslave.py | 7 ++-- tiramisu/setting.py | 55 +++++++++++++++++++++-------- 5 files changed, 154 insertions(+), 36 deletions(-) diff --git a/test/test_option_setting.py b/test/test_option_setting.py index cf6914d..fe609e5 100644 --- a/test/test_option_setting.py +++ b/test/test_option_setting.py @@ -4,6 +4,8 @@ do_autopath() from py.test import raises +from tiramisu.i18n import _ +from tiramisu.error import display_list from tiramisu.setting import owners, groups from tiramisu.config import Config from tiramisu.option import ChoiceOption, BoolOption, IntOption, FloatOption, \ @@ -471,3 +473,65 @@ def test_reset_properties_force_store_value(): setting.reset(all_properties=True) assert setting._p_.get_modified_properties() == {} raises(ValueError, 'setting.reset(all_properties=True, opt=option)') + + +def test_pprint(): + msg_error = _('cannot access to {} {} because has {} {}') + msg_is = _("the value of {0} is {1}") + msg_is_not = _("the value of {0} is not {1}") + + s = StrOption("string", "", default=["string"], default_multi="string", multi=True, properties=('hidden', 'disabled')) + s2 = StrOption("string2", "", default="string") + s3 = StrOption("string3", "", default=["string"], default_multi="string", multi=True, properties=('hidden',)) + intoption = IntOption('int', 'Test int option', default=0) + stroption = StrOption('str', 'Test string option', default="abc", + requires=[{'option': intoption, 'expected': 2, 'action': 'hidden', 'inverse': True}, + {'option': intoption, 'expected': 3, 'action': 'hidden', 'inverse': True}, + {'option': intoption, 'expected': 4, 'action': 'hidden', 'inverse': True}, + {'option': intoption, 'expected': 1, 'action': 'disabled'}, + {'option': s2, 'expected': 'string', 'action': 'disabled'}]) + + val2 = StrOption('val2', "") + descr2 = OptionDescription("options", "", [val2], requires=[{'option': intoption, 'expected': 1, 'action': 'hidden'}]) + + val3 = StrOption('val3', "", requires=[{'option': stroption, 'expected': '2', 'action': 'hidden', 'inverse': True}]) + + descr = OptionDescription("options", "", [s, s2, s3, intoption, stroption, descr2, val3]) + config = Config(descr) + setting = config.cfgimpl_get_settings() + config.read_write() + config.int = 1 + try: + config.str + except Exception, err: + pass + + assert str(err) == msg_error.format('option', 'str', 'properties', display_list(['disabled (' + display_list([msg_is.format('int', '1'), msg_is.format('string2', 'string')]) + ')', 'hidden (' + msg_is_not.format('int', display_list([2, 3, 4], 'or')) + ')'])) + + try: + config.options.val2 + except Exception, err: + pass + + assert str(err) == msg_error.format('optiondescription', 'options', 'property', 'hidden (' + msg_is.format('int', 1) + ')') + + try: + config.val3 + except Exception, err: + pass + + assert str(err) == msg_error.format('option', 'val3', 'property', 'hidden (' + display_list([msg_is.format('int', 1), msg_is.format('string2', 'string'), msg_is_not.format('int', display_list([2, 3, 4], 'or'))]) + ')') + + try: + config.string + except Exception, err: + pass + + assert str(err) == msg_error.format('option', 'string', 'properties', display_list(['disabled', 'hidden'])) + + try: + config.string3 + except Exception, err: + pass + + assert str(err) == msg_error.format('option', 'string3', 'property', 'hidden') diff --git a/tiramisu/error.py b/tiramisu/error.py index 163c7c3..aa035b1 100644 --- a/tiramisu/error.py +++ b/tiramisu/error.py @@ -15,15 +15,45 @@ # along with this program. If not, see . # ____________________________________________________________ "user defined exceptions" +from .i18n import _ + + +def display_list(lst, separator='and'): + if len(lst) == 1: + return lst[0] + else: + lst_ = [] + for l in lst[:-1]: + lst_.append(str(l)) + return ', '.join(lst_) + _(' {} ').format(separator) + str(lst[-1]) # Exceptions for an Option class PropertiesOptionError(AttributeError): "attempt to access to an option with a property that is not allowed" - def __init__(self, msg, proptype): + def __init__(self, msg, proptype, settings, datas, option_type): self.proptype = proptype + self._settings = settings + self._datas = datas + self._type = option_type super(PropertiesOptionError, self).__init__(msg) + def __str__(self): + #this part is a bit slow, so only execute when display + req = self._settings.apply_requires(**self._datas) + if req != {}: + msg = [] + for action, msg_ in req.items(): + msg.append('{0} ({1})'.format(action, display_list(msg_))) + if len(req) == 1: + prop_msg = _('property') + else: + prop_msg = _('properties') + msg = display_list(msg) + return _('cannot access to {0} {1} because has {2} {3}').format(self._type, self._datas['path'], prop_msg, msg) + else: + return self.message + #____________________________________________________________ # Exceptions for a Config diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index 4b0386b..d05358c 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -24,9 +24,10 @@ import warnings import sys from ..i18n import _ -from ..setting import log, undefined +from ..setting import log, undefined, debug from ..autolib import carry_out_calculation -from ..error import ConfigError, ValueWarning, PropertiesOptionError +from ..error import (ConfigError, ValueWarning, PropertiesOptionError, + display_list) from ..storage import get_storages_option @@ -38,13 +39,6 @@ forbidden_names = frozenset(['iter_all', 'iter_group', 'find', 'find_first', 'read_write', 'getowner', 'set_contexts']) -def display_list(list_): - if len(list_) == 1: - return list_[0] - else: - return ', '.join(list_[:-1]) + _(' and ') + list_[-1] - - 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 @@ -417,7 +411,8 @@ class Option(OnlyOption): returns_raise=True) if isinstance(opt_value, Exception): if isinstance(opt_value, PropertiesOptionError): - log.debug('propertyerror in _launch_consistency: {0}'.format(opt_value)) + if debug: + log.debug('propertyerror in _launch_consistency: {0}'.format(opt_value)) if transitive: return opt_value else: @@ -493,10 +488,11 @@ class Option(OnlyOption): err = self._validate(_value, context, current_opt, returns_raise=True) if err: - log.debug('do_validation: value: {0}, index: {1}, ' - 'submulti_index: {2}'.format(_value, _index, - submulti_index), - exc_info=True) + if debug: + log.debug('do_validation: value: {0}, index: {1}, ' + 'submulti_index: {2}'.format(_value, _index, + submulti_index), + exc_info=True) err_msg = '{0}'.format(err) if err_msg: msg = _('{0} is an invalid {1} for option {2}, {3}' @@ -512,8 +508,9 @@ class Option(OnlyOption): if not error: error = self._second_level_validation(_value, self._is_warnings_only()) if error: - log.debug(_('do_validation for {0}: error in value').format( - self.impl_getname()), exc_info=True) + if debug: + log.debug(_('do_validation for {0}: error in value').format( + self.impl_getname()), exc_info=True) if self._is_warnings_only(): warning = error error = None @@ -718,7 +715,8 @@ class Option(OnlyOption): msg = _("value for {0} and {1} should be different") else: msg = _("value for {0} and {1} must be different") - log.debug('_cons_not_equal: {0} and {1} are not different'.format(val_inf, val_sup)) + if debug: + log.debug('_cons_not_equal: {0} and {1} are not different'.format(val_inf, val_sup)) return ValueError(msg.format(opts[idx_inf].impl_getname(), opts[idx_inf + idx_sup + 1].impl_getname())) diff --git a/tiramisu/option/masterslave.py b/tiramisu/option/masterslave.py index 7f5e048..ca12351 100644 --- a/tiramisu/option/masterslave.py +++ b/tiramisu/option/masterslave.py @@ -20,7 +20,7 @@ # the whole pypy projet is under MIT licence # ____________________________________________________________ from ..i18n import _ -from ..setting import log, undefined +from ..setting import log, undefined, debug from ..error import SlaveError, PropertiesOptionError from .baseoption import DynSymLinkOption, SymLinkOption, Option @@ -278,8 +278,9 @@ class MasterSlaves(object): def validate_slave_length(self, masterlen, valuelen, name, opt, setitem=False): if valuelen > masterlen or (valuelen < masterlen and setitem): # pragma: optional cover - log.debug('validate_slave_length: masterlen: {0}, valuelen: {1}, ' - 'setitem: {2}'.format(masterlen, valuelen, setitem)) + if debug: + log.debug('validate_slave_length: masterlen: {0}, valuelen: {1}, ' + 'setitem: {2}'.format(masterlen, valuelen, setitem)) raise SlaveError(_("invalid len for the slave: {0}" " which has {1} as master").format( name, self.getmaster(opt).impl_getname())) diff --git a/tiramisu/setting.py b/tiramisu/setting.py index bb9829e..04b5b3b 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -20,7 +20,7 @@ from copy import copy from logging import getLogger import weakref from .error import (RequirementError, PropertiesOptionError, - ConstError, ConfigError) + ConstError, ConfigError, display_list) from .i18n import _ @@ -113,6 +113,7 @@ log = getLogger('tiramisu') #FIXME #import logging #logging.basicConfig(format='%(levelname)s:%(message)s', level=logging.DEBUG) +debug = False # ____________________________________________________________ @@ -394,7 +395,7 @@ class Settings(object): if opt.impl_is_multi() and not opt.impl_is_master_slaves('slave'): props.add('empty') if apply_requires: - requires = self.apply_requires(opt, path, setting_properties, index) + requires = self.apply_requires(opt, path, setting_properties, index, False) if requires != set([]): props = copy(props) props |= requires @@ -441,7 +442,7 @@ class Settings(object): value=None, force_permissive=False, setting_properties=undefined, self_properties=undefined, - index=None): + index=None, debug=False): """ validation upon the properties related to `opt_or_descr` @@ -496,22 +497,30 @@ class Settings(object): # at this point an option should not remain in properties if properties != frozenset(): props = list(properties) + datas = {'opt': opt_or_descr, 'path': path, 'setting_properties': setting_properties, + 'index': index, 'debug': True} + if is_descr: + opt_type = 'optiondescription' + else: + opt_type = 'option' if 'frozen' in properties: return PropertiesOptionError(_('cannot change the value for ' 'option {0} this option is' ' frozen').format( opt_or_descr.impl_getname()), - props) + props, self, datas, opt_type) else: - if is_descr: - opt_type = 'optiondescription' + if len(props) == 1: + prop_msg = 'property' else: - opt_type = 'option' - return PropertiesOptionError(_("trying to access to an {0} " - "named: {1} with properties {2}" + prop_msg = 'properties' + return PropertiesOptionError(_("cannot access to {0} {1} " + "because has {2} {3}" "").format(opt_type, opt_or_descr._name, - str(props)), props) + prop_msg, + display_list(props)), props, + self, datas, opt_type) def setpermissive(self, permissive, opt=None, path=None): """ @@ -571,7 +580,7 @@ class Settings(object): else: self._p_.reset_all_cache() - def apply_requires(self, opt, path, setting_properties, index): + def apply_requires(self, opt, path, setting_properties, index, debug): """carries out the jit (just in time) requirements between options a requirement is a tuple of this form that comes from the option's @@ -619,7 +628,10 @@ class Settings(object): return frozenset() # filters the callbacks - calc_properties = set() + if debug: + calc_properties = {} + else: + calc_properties = set() context = self._getcontext() for requires in opt.impl_getrequires(): for require in requires: @@ -650,17 +662,30 @@ class Settings(object): "{1} {2}").format(opt._name, reqpath, properties)) + orig_value = value # transitive action, force expected value = expected[0] inverse = False else: raise value + else: + orig_value = value if (not inverse and value in expected or inverse and value not in expected): - calc_properties.add(action) - # the calculation cannot be carried out - break + if debug: + if isinstance(orig_value, PropertiesOptionError): + for act, msg in orig_value._settings.apply_requires(**orig_value._datas).items(): + calc_properties.setdefault(action, []).extend(msg) + else: + if not inverse: + msg = _("the value of {0} is {1}") + else: + msg = _("the value of {0} is not {1}") + calc_properties.setdefault(action, []).append(msg.format(reqpath, display_list(expected, 'or'))) + else: + calc_properties.add(action) + break return calc_properties def get_modified_properties(self):