From: W. Trevor King Date: Thu, 25 Jun 2009 11:45:57 +0000 (-0400) Subject: tag --remove now returns bug.settings["extra_strings"] to EMPTY. X-Git-Tag: 1.0.0~70^2^2~4 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=797bf225ff29a73fa0770d6cefef12a27cc3d760;p=be.git tag --remove now returns bug.settings["extra_strings"] to EMPTY. extra_strings returns to a defaulting property from a cached/generator property, with the help of the new, mutable defaults. Lots of deepcopies avoid mutable default uncertainty too ;). And copy.deepcopy([]) should be pretty cheap. tag --remove had previously left settings["extra_strings"] as [], which polluted the bug's values file. Now the improved defaulting_property notices a return to the default [], and sets the internally stored value to EMPTY. I struggled with creating a more intuitive way to notice changes to extra_strings than the tmp = bug.extra_strings bug.extra_strings = tmp but didn't have any luck. The problem seems to be that if you only hand out copies of your default, you don't have any pointers to what you handed out to check for changes. On the other hand, if you hand out your original default, any external changes will _change_ your original default. I suppose you could only hand out copies, but keep a list of all copies handed out, but that sounds like a disaster. Reassigning is easy enough. --- diff --git a/becommands/tag.py b/becommands/tag.py index 0b19e91..b8030f3 100644 --- a/becommands/tag.py +++ b/becommands/tag.py @@ -61,6 +61,11 @@ def execute(args, test=False): >>> a = bd.bug_from_shortname("a") >>> print a.extra_strings [] + >>> execute(["a", "Alphabetically first"], test=True) + Tagging bug a: + Alphabetically first + >>> execute(["--remove", "a", "Alphabetically first"], test=True) + Tags for a: """ parser = get_parser() options, args = parser.parse_args(args) @@ -79,7 +84,6 @@ def execute(args, test=False): new_tag = None if len(args) == 2: given_tag = args[1] - # reassign list so the change_hook realizes we've altered it. tags = bug.extra_strings tag_string = "TAG:%s" % given_tag if options.remove == True: @@ -87,15 +91,15 @@ def execute(args, test=False): else: # add the tag new_tag = given_tag tags.append(tag_string) - bug.extra_strings = tags + bug.extra_strings = tags # reassign to notice change + + bug.save() tags = [] for estr in bug.extra_strings: if estr.startswith("TAG:"): tags.append(estr[4:]) - bd.save() - if new_tag == None: print "Tags for %s:" % bug.uuid else: diff --git a/libbe/bug.py b/libbe/bug.py index 0f912cf..7418933 100644 --- a/libbe/bug.py +++ b/libbe/bug.py @@ -185,11 +185,11 @@ class Bug(settings_object.SavedSettingsObject): fset=_set_time, doc="An integer version of .time_string") - def _extra_strings_generator(self): - return [] def _extra_strings_check_fn(value): "Require an iterable full of strings" - if not hasattr(value, "__iter__"): + if value == settings_object.EMPTY: + return True + elif not hasattr(value, "__iter__"): return False for x in value: if type(x) not in types.StringTypes: @@ -200,7 +200,7 @@ class Bug(settings_object.SavedSettingsObject): self._prop_save_settings(old, new) @_versioned_property(name="extra_strings", doc="Space for an array of extra strings. Useful for storing state for functionality implemented purely in becommands/.py.", - generator=_extra_strings_generator, + default=[], check_fn=_extra_strings_check_fn, change_hook=_extra_strings_change_hook, mutable=True) diff --git a/libbe/properties.py b/libbe/properties.py index 2ccc50b..956ecc3 100644 --- a/libbe/properties.py +++ b/libbe/properties.py @@ -71,7 +71,7 @@ def doc_property(doc=None): return funcs return decorator -def local_property(name, null=None): +def local_property(name, null=None, mutable_null=False): """ Define get/set access to per-parent-instance local storage. Uses .__value to store the value for a particular owner instance. @@ -85,7 +85,11 @@ def local_property(name, null=None): def _fget(self): if fget is not None: fget(self) - value = getattr(self, "_%s_value" % name, null) + if mutable_null == True: + ret_null = copy.deepcopy(null) + else: + ret_null = null + value = getattr(self, "_%s_value" % name, ret_null) return value def _fset(self, value): setattr(self, "_%s_value" % name, value) @@ -123,7 +127,46 @@ def settings_property(name, null=None): return funcs return decorator -def defaulting_property(default=None, null=None): + +# Allow comparison and caching with _original_ values for mutables, +# since +# +# >>> a = [] +# >>> b = a +# >>> b.append(1) +# >>> a +# [1] +# >>> a==b +# True +def _hash_mutable_value(value): + return repr(value) +def _init_mutable_property_cache(self): + if not hasattr(self, "_mutable_property_cache_hash"): + # first call to _fget for any mutable property + self._mutable_property_cache_hash = {} + self._mutable_property_cache_copy = {} +def _set_cached_mutable_property(self, cacher_name, property_name, value): + _init_mutable_property_cache(self) + self._mutable_property_cache_hash[(cacher_name, property_name)] = \ + _hash_mutable_value(value) + self._mutable_property_cache_copy[(cacher_name, property_name)] = \ + copy.deepcopy(value) +def _get_cached_mutable_property(self, cacher_name, property_name, default=None): + _init_mutable_property_cache(self) + if (cacher_name, property_name) not in self._mutable_property_cache_copy: + return default + return self._mutable_property_cache_copy[(cacher_name, property_name)] +def _cmp_cached_mutable_property(self, cacher_name, property_name, value): + _init_mutable_property_cache(self) + if (cacher_name, property_name) not in self._mutable_property_cache_hash: + return 1 # any value > non-existant old hash + old_hash = self._mutable_property_cache_hash[(cacher_name, property_name)] + return cmp(_hash_mutable_value(value), old_hash) + + +def defaulting_property(default=None, null=None, + default_mutable=False, + null_mutable=False): """ Define a default value for get access to a property. If the stored value is null, then default is returned. @@ -133,14 +176,21 @@ def defaulting_property(default=None, null=None): funcs = funcs() fget = funcs.get("fget") fset = funcs.get("fset") + name = funcs.get("name", "") def _fget(self): value = fget(self) if value == null: - return default + if default_mutable == True: + return copy.deepcopy(default) + else: + return default return value def _fset(self, value): if value == default: - value = null + if null_mutable == True: + value = copy.deepcopy(null) + else: + value = null fset(self, value) funcs["fget"] = _fget funcs["fset"] = _fset @@ -195,7 +245,7 @@ def checked_property(allowed=[]): return funcs return decorator -def cached_property(generator, initVal=None): +def cached_property(generator, initVal=None, mutable=False): """ Allow caching of values generated by generator(instance), where instance is the instance to which this property belongs. Uses @@ -204,7 +254,7 @@ def cached_property(generator, initVal=None): When the cache flag is True or missing and the stored value is initVal, the first fget call triggers the generator function, - whiose output is stored in __cached_value. That and + whose output is stored in __cached_value. That and subsequent calls to fget will return this cached value. If the input value is no longer initVal (e.g. a value has been @@ -216,25 +266,27 @@ def cached_property(generator, initVal=None): The cache flag is missing on initialization. Particular instances may override by setting their own flag. + + In the case that mutable == True, all caching is disabled and the + generator is called whenever the cached value would otherwise be + used. This avoids uncertainties in the value of stored mutables. """ def decorator(funcs): if hasattr(funcs, "__call__"): funcs = funcs() fget = funcs.get("fget") - fset = funcs.get("fset") name = funcs.get("name", "") def _fget(self): cache = getattr(self, "_%s_cache" % name, True) value = fget(self) - if cache == True: - if value == initVal: + if value == initVal: + if cache == True and mutable == False: if hasattr(self, "_%s_cached_value" % name): value = getattr(self, "_%s_cached_value" % name) else: value = generator(self) setattr(self, "_%s_cached_value" % name, value) - else: - if value == initVal: + else: value = generator(self) return value funcs["fget"] = _fget @@ -278,20 +330,6 @@ def change_hook_property(hook, mutable=False): called _after_ the new value has been stored, allowing you to change the stored value if you want. - If mutable=True, store a string-representation of the old_value - for use in comparisions, since - - >>> a = [] - >>> b = a - >>> b.append(1) - >>> a - [1] - >>> a==b - True - - The string-value-changed test may miss the first write, since - there will not have been an opportunity to cache a string version - of the old value. """ def decorator(funcs): if hasattr(funcs, "__call__"): @@ -299,37 +337,16 @@ def change_hook_property(hook, mutable=False): fget = funcs.get("fget") fset = funcs.get("fset") name = funcs.get("name", "") - def hash_value(value): # only used if mutable == True - return repr(value) def _fget(self, new_value=None, from_fset=False): # only used if mutable == True value = fget(self) - if not hasattr(self, "_change_hook_property_mutable_cache_hash"): - # first call to _fget for any mutable property - self._change_hook_property_mutable_cache_hash = {} - self._change_hook_property_mutable_cache_copy = {} - if name not in self._change_hook_property_mutable_cache_hash: - # first call to _fget for this particular mutable property - self._change_hook_property_mutable_cache_hash[name] = \ - hash_value(new_value) - self._change_hook_property_mutable_cache_copy[name] = \ - copy.deepcopy(new_value) - elif from_fset == True: # return cached value, and cache new value - old_hash = self._change_hook_property_mutable_cache_hash[name] - if hash_value(value) != old_hash: - value = self._change_hook_property_mutable_cache_copy[name] - self._change_hook_property_mutable_cache_hash[name] = \ - hash_value(new_value) - self._change_hook_property_mutable_cache_copy[name] = \ - copy.deepcopy(new_value) - else: # check for a change in value while we weren't looking - old_hash = self._change_hook_property_mutable_cache_hash[name] - if hash_value(value) != old_hash: - old_val=self._change_hook_property_mutable_cache_copy[name] - self._change_hook_property_mutable_cache_hash[name] = \ - hash_value(value) - self._change_hook_property_mutable_cache_copy[name] = \ - copy.deepcopy(value) - hook(self, old_val, value) + if _cmp_cached_mutable_property(self, "change hook property", name, value) != 0: + # there has been a change, cache new value + old_value = _get_cached_mutable_property(self, "change hook property", name) + _set_cached_mutable_property(self, "change hook property", name, value) + if from_fset == True: # return previously cached value + value = old_value + else: # the value changed while we weren't looking + hook(self, old_value, value) return value def _fset(self, value): if mutable == True: # get cached previous value @@ -344,7 +361,7 @@ def change_hook_property(hook, mutable=False): funcs["fset"] = _fset return funcs return decorator - + class DecoratorTests(unittest.TestCase): def testLocalDoc(self): @@ -385,16 +402,18 @@ class DecoratorTests(unittest.TestCase): class Test(object): @Property @defaulting_property(default='y', null='x') - @local_property(name="DEFAULT") + @local_property(name="DEFAULT", null=5) def x(): return {} t = Test() - self.failUnless(t.x == None, str(t.x)) + self.failUnless(t.x == 5, str(t.x)) t.x = 'x' self.failUnless(t.x == 'y', str(t.x)) t.x = 'y' self.failUnless(t.x == 'y', str(t.x)) t.x = 'z' self.failUnless(t.x == 'z', str(t.x)) + t.x = 5 + self.failUnless(t.x == 5, str(t.x)) def testCheckedLocalProperty(self): class Test(object): @Property @@ -574,21 +593,21 @@ class DecoratorTests(unittest.TestCase): a = t.x self.failUnless(t.old == [], t.old) self.failUnless(t.new == [5], t.new) - self.failUnless(t.hook_calls == 5, t.hook_calls) + self.failUnless(t.hook_calls == 6, t.hook_calls) t.x.append(6) # this append(6) is not noticed yet self.failUnless(t.old == [], t.old) self.failUnless(t.new == [5,6], t.new) - self.failUnless(t.hook_calls == 5, t.hook_calls) + self.failUnless(t.hook_calls == 6, t.hook_calls) # this append(7) is not noticed, but the t.x get causes the # append(6) to be noticed t.x.append(7) self.failUnless(t.old == [5], t.old) self.failUnless(t.new == [5,6,7], t.new) - self.failUnless(t.hook_calls == 6, t.hook_calls) + self.failUnless(t.hook_calls == 7, t.hook_calls) a = t.x # now the append(7) is noticed self.failUnless(t.old == [5,6], t.old) self.failUnless(t.new == [5,6,7], t.new) - self.failUnless(t.hook_calls == 7, t.hook_calls) + self.failUnless(t.hook_calls == 8, t.hook_calls) suite = unittest.TestLoader().loadTestsFromTestCase(DecoratorTests) diff --git a/libbe/settings_object.py b/libbe/settings_object.py index a854ce5..60fddb9 100644 --- a/libbe/settings_object.py +++ b/libbe/settings_object.py @@ -126,10 +126,12 @@ def versioned_property(name, doc, def decorator(funcs): fulldoc = doc if default != None: - defaulting = defaulting_property(default=default, null=EMPTY) + defaulting = defaulting_property(default=default, null=EMPTY, + default_mutable=mutable) fulldoc += "\n\nThis property defaults to %s" % default if generator != None: - cached = cached_property(generator=generator, initVal=EMPTY) + cached = cached_property(generator=generator, initVal=EMPTY, + mutable=mutable) fulldoc += "\n\nThis property is generated with %s" % generator if check_fn != None: fn_checked = fn_checked_property(value_allowed_fn=check_fn)