diff --git a/news/util-standards.rst b/news/util-standards.rst new file mode 100644 index 00000000..84ed5a9b --- /dev/null +++ b/news/util-standards.rst @@ -0,0 +1,31 @@ +**Added:** + +* Add ``add_observer``, ``remove_observer`` and ``has_observer`` to + ``Observable``, and ``has_tags`` and ``verify_tags`` to ``TagManager``. +* Add ``is_identifier`` and ``validate_name`` to ``diffpy.srfit.util.nameutils``, + ``convert_input_to_string`` to ``diffpy.srfit.util.inpututils``, and + ``sort_key_for_numeric_string`` to ``diffpy.srfit.util``. + +**Changed:** + +* The error raised for an invalid name now says how to fix it. + +**Deprecated:** + +* ``Observable.addObserver``, ``removeObserver`` and ``hasObserver``, + ``TagManager.hasTags`` and ``verifyTags``, ``isIdentifier``, + ``validateName``, ``inputToString`` and ``sortKeyForNumericString`` are + deprecated in favour of their snake_case replacements. The old names still + work and warn, and will be removed in version 4.0.0. + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/srfit/equation/equationmod.py b/src/diffpy/srfit/equation/equationmod.py index 44bcaf01..fa5918d0 100644 --- a/src/diffpy/srfit/equation/equationmod.py +++ b/src/diffpy/srfit/equation/equationmod.py @@ -161,11 +161,11 @@ def setRoot(self, root): # Stop observing the old root if self.root is not None: - self.root.removeObserver(self._flush) + self.root.remove_observer(self._flush) # Add the new root self.root = root - self.root.addObserver(self._flush) + self.root.add_observer(self._flush) self._flush(other=(self,)) # Get the args diff --git a/src/diffpy/srfit/equation/literals/operators.py b/src/diffpy/srfit/equation/literals/operators.py index 917a17ae..810dd79e 100644 --- a/src/diffpy/srfit/equation/literals/operators.py +++ b/src/diffpy/srfit/equation/literals/operators.py @@ -115,7 +115,7 @@ def addLiteral(self, literal): # Make sure we don't have self-reference self._loop_check(literal) self.args.append(literal) - literal.addObserver(self._flush) + literal.add_observer(self._flush) self._flush(other=(self,)) return diff --git a/src/diffpy/srfit/equation/visitors/swapper.py b/src/diffpy/srfit/equation/visitors/swapper.py index 5c76323b..6d957611 100644 --- a/src/diffpy/srfit/equation/visitors/swapper.py +++ b/src/diffpy/srfit/equation/visitors/swapper.py @@ -97,7 +97,7 @@ def onOperator(self, op): # attempt to remove the same observer more than once, which # might happen if the oldlit appears multiple times in op.args. try: - oldlit.removeObserver(op._flush) + oldlit.remove_observer(op._flush) except KeyError: pass @@ -108,12 +108,12 @@ def onOperator(self, op): except ValueError: # Restore the old literal op.args.insert(idx, oldlit) - oldlit.addObserver(op._flush) + oldlit.add_observer(op._flush) raise # If we got here, then go on with replacing the literal op.args.insert(idx, newlit) - newlit.addObserver(op._flush) + newlit.add_observer(op._flush) op._flush(other=()) self._swap = False diff --git a/src/diffpy/srfit/fitbase/fithook.py b/src/diffpy/srfit/fitbase/fithook.py index 6edae312..c9d4f5ce 100644 --- a/src/diffpy/srfit/fitbase/fithook.py +++ b/src/diffpy/srfit/fitbase/fithook.py @@ -31,7 +31,7 @@ import numpy -from diffpy.srfit.util import sortKeyForNumericString +from diffpy.srfit.util import sort_key_for_numeric_string class FitHook(object): @@ -178,7 +178,7 @@ def postcall(self, recipe, chiv): def _byname(nv): - return sortKeyForNumericString(nv[0]) + return sort_key_for_numeric_string(nv[0]) # End class PrintFitHook diff --git a/src/diffpy/srfit/fitbase/fitrecipe.py b/src/diffpy/srfit/fitbase/fitrecipe.py index 4610c56c..78c6a516 100644 --- a/src/diffpy/srfit/fitbase/fitrecipe.py +++ b/src/diffpy/srfit/fitbase/fitrecipe.py @@ -1131,7 +1131,7 @@ def is_free(self, var): bool True if the variable is free (not fixed), False otherwise. """ - return not self._tagmanager.hasTags(var, self._fixedtag) + return not self._tagmanager.has_tags(var, self._fixedtag) @deprecated(isFree_dep_msg) def isFree(self, var): diff --git a/src/diffpy/srfit/fitbase/fitresults.py b/src/diffpy/srfit/fitbase/fitresults.py index 9d9da031..e315fa50 100644 --- a/src/diffpy/srfit/fitbase/fitresults.py +++ b/src/diffpy/srfit/fitbase/fitresults.py @@ -30,8 +30,8 @@ import numpy from diffpy.srfit.util import _DASHEDLINE -from diffpy.srfit.util import sortKeyForNumericString as numstr -from diffpy.srfit.util.inpututils import inputToString +from diffpy.srfit.util import sort_key_for_numeric_string as numstr +from diffpy.srfit.util.inpututils import convert_input_to_string from diffpy.utils._deprecator import build_deprecation_message, deprecated fitresults_base = "diffpy.srfit.fitbase.FitResults" @@ -855,7 +855,7 @@ def resultsDictionary(results): def _parse_results_dictionary(results): """Remove this function when when resultsDictionary and initializeRecipe are removed in 4.0.0.""" - resstr = inputToString(results) + resstr = convert_input_to_string(results) rx = { "f": r"[+-]? *(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?", "n": r"[a-zA-Z_]\w*", diff --git a/src/diffpy/srfit/fitbase/parameter.py b/src/diffpy/srfit/fitbase/parameter.py index 2da0d2f4..f338e023 100644 --- a/src/diffpy/srfit/fitbase/parameter.py +++ b/src/diffpy/srfit/fitbase/parameter.py @@ -33,7 +33,7 @@ from diffpy.srfit.fitbase.validatable import Validatable from diffpy.srfit.interface import _parameter_interface from diffpy.srfit.util.argbinders import bind2nd -from diffpy.srfit.util.nameutils import validateName +from diffpy.srfit.util.nameutils import validate_name from diffpy.utils._deprecator import build_deprecation_message, deprecated parameter_base = "diffpy.srfit.fitbase.Parameter" @@ -102,7 +102,7 @@ def __init__(self, name, value=None, const=False): """ self.constrained = False self.bounds = [-numpy.inf, +numpy.inf] - validateName(name) + validate_name(name) Argument.__init__(self, name, value, const) return @@ -287,7 +287,7 @@ def __init__(self, name, par): ValueError If the name is not a valid attribute identifier. """ - validateName(name) + validate_name(name) self.name = name self.par = par diff --git a/src/diffpy/srfit/fitbase/profile.py b/src/diffpy/srfit/fitbase/profile.py index c9b6f931..420deb54 100644 --- a/src/diffpy/srfit/fitbase/profile.py +++ b/src/diffpy/srfit/fitbase/profile.py @@ -125,9 +125,9 @@ def __init__(self): self.meta = {} # Observable - self.xpar.addObserver(self._flush) - self.ypar.addObserver(self._flush) - self.dypar.addObserver(self._flush) + self.xpar.add_observer(self._flush) + self.ypar.add_observer(self._flush) + self.dypar.add_observer(self._flush) return # We want x, y, ycalc and dy to stay in-sync with xpar, ypar and dypar diff --git a/src/diffpy/srfit/fitbase/profilegenerator.py b/src/diffpy/srfit/fitbase/profilegenerator.py index 0e73347b..597d5437 100644 --- a/src/diffpy/srfit/fitbase/profilegenerator.py +++ b/src/diffpy/srfit/fitbase/profilegenerator.py @@ -174,10 +174,10 @@ def set_profile(self, profile): will store the calculated signal. """ if self.profile is not None: - self.profile.removeObserver(self._on_profile_update) + self.profile.remove_observer(self._on_profile_update) self.profile = profile - self.profile.addObserver(self._on_profile_update) + self.profile.add_observer(self._on_profile_update) self._on_profile_update(other=(self,)) return diff --git a/src/diffpy/srfit/fitbase/recipeorganizer.py b/src/diffpy/srfit/fitbase/recipeorganizer.py index caa28aed..0e4947f4 100644 --- a/src/diffpy/srfit/fitbase/recipeorganizer.py +++ b/src/diffpy/srfit/fitbase/recipeorganizer.py @@ -38,8 +38,8 @@ from diffpy.srfit.fitbase.validatable import Validatable from diffpy.srfit.interface import _recipeorganizer_interface from diffpy.srfit.util import _DASHEDLINE -from diffpy.srfit.util import sortKeyForNumericString as numstr -from diffpy.srfit.util.nameutils import validateName +from diffpy.srfit.util import sort_key_for_numeric_string as numstr +from diffpy.srfit.util.nameutils import validate_name from diffpy.srfit.util.observable import Observable from diffpy.utils._deprecator import build_deprecation_message, deprecated @@ -218,7 +218,7 @@ class RecipeContainer(Observable, Configurable, Validatable): def __init__(self, name): Observable.__init__(self) Configurable.__init__(self) - validateName(name) + validate_name(name) self.name = name self._parameters = OrderedDict() @@ -615,13 +615,13 @@ def _add_object(self, obj, d, check=True): # Detach the old object, if there is one if oldobj is not None: - oldobj.removeObserver(self._flush) + oldobj.remove_observer(self._flush) # Add the object d[obj.name] = obj # Observe the object - obj.addObserver(self._flush) + obj.add_observer(self._flush) # Store this as a configurable object self._store_configurable(obj) @@ -640,7 +640,7 @@ def _remove_object(self, obj, d): raise ValueError(m) del d[obj.name] - obj.removeObserver(self._flush) + obj.remove_observer(self._flush) return diff --git a/src/diffpy/srfit/structure/objcrystparset.py b/src/diffpy/srfit/structure/objcrystparset.py index 3ccf9934..fc09ba7b 100644 --- a/src/diffpy/srfit/structure/objcrystparset.py +++ b/src/diffpy/srfit/structure/objcrystparset.py @@ -1220,9 +1220,9 @@ def addAtoms(self, atomlist): self.matoms.update(atomlist) # Make sure we're observing these atoms for a in atomlist: - a.x.addObserver(self._flush) - a.y.addObserver(self._flush) - a.z.addObserver(self._flush) + a.x.add_observer(self._flush) + a.y.add_observer(self._flush) + a.z.add_observer(self._flush) # Record the added atoms in the StretchMode scatlist = [a.scat for a in atomlist] @@ -1349,9 +1349,9 @@ def __init__(self, name, atom1, atom2, value=None, const=False, mode=None): # Observe the atom positions for a in [atom1, atom2]: - a.x.addObserver(self._flush) - a.y.addObserver(self._flush) - a.z.addObserver(self._flush) + a.x.add_observer(self._flush) + a.y.add_observer(self._flush) + a.z.add_observer(self._flush) self.atom1 = atom1 self.atom2 = atom2 @@ -1510,9 +1510,9 @@ def __init__( # Observe the atom positions for a in [atom1, atom2, atom3]: - a.x.addObserver(self._flush) - a.y.addObserver(self._flush) - a.z.addObserver(self._flush) + a.x.add_observer(self._flush) + a.y.add_observer(self._flush) + a.z.add_observer(self._flush) self.atom1 = atom1 self.atom2 = atom2 @@ -1688,9 +1688,9 @@ def __init__( # Observe the atom positions for a in [atom1, atom2, atom3, atom4]: - a.x.addObserver(self._flush) - a.y.addObserver(self._flush) - a.z.addObserver(self._flush) + a.x.add_observer(self._flush) + a.y.add_observer(self._flush) + a.z.add_observer(self._flush) self.atom1 = atom1 self.atom2 = atom2 diff --git a/src/diffpy/srfit/util/__init__.py b/src/diffpy/srfit/util/__init__.py index c8c9b830..ec13776a 100644 --- a/src/diffpy/srfit/util/__init__.py +++ b/src/diffpy/srfit/util/__init__.py @@ -14,37 +14,55 @@ ############################################################################## """Utilities and constants used throughout SrFit.""" +import re + +from diffpy.utils._deprecator import build_deprecation_message, deprecated + _DASHEDLINE = 78 * "-" +sortKeyForNumericString_dep_msg = build_deprecation_message( + "diffpy.srfit.util", + "sortKeyForNumericString", + "sort_key_for_numeric_string", + "4.0.0", +) -def sortKeyForNumericString(s): - """Compute key for sorting strings according to their integer - numeric value. - Each string gets split to string and integer segments to create keys - for comparison. Signs, decimal points and exponents are ignored. - This function is intended as the ``key`` argument for the ``sorted`` - or ``list.sort`` function. +def sort_key_for_numeric_string(string): + """Return a sort key that orders the numbers in a string by value. + + The string is split into text and integer segments, so ``"a2"`` sorts + before ``"a10"``. Signs, decimal points and exponents are ignored. Use it + as the ``key`` argument of ``sorted`` or ``list.sort``. Parameters ---------- - s : str - String which may have numeric components, e.g., "a12b". + string : str + The string to build a key for, which may contain numbers, e.g. + ``"a12b"``. Returns ------- tuple - Tuple of non-numeric segments intermixed with integer values. + The text segments of ``string`` with integer values in between, + e.g. ``("a", 12, "b")``. """ - if sortKeyForNumericString._rx is None: - import re + segments = re.split(r"(\d+)", string) + sort_key = tuple( + int(segment) if segment.isdecimal() else segment + for segment in segments + ) + return sort_key - sortKeyForNumericString._rx = re.compile(r"(\d+)") - rx = sortKeyForNumericString._rx - rv = tuple((int(w) if i % 2 else w) for i, w in enumerate(rx.split(s))) - return rv +@deprecated(sortKeyForNumericString_dep_msg) +def sortKeyForNumericString(s): + """This function has been deprecated and will be removed in version + 4.0.0. + + Please use diffpy.srfit.util.sort_key_for_numeric_string instead. + """ + return sort_key_for_numeric_string(s) -sortKeyForNumericString._rx = None # End of file diff --git a/src/diffpy/srfit/util/argbinders.py b/src/diffpy/srfit/util/argbinders.py index 4c896308..889a38b5 100644 --- a/src/diffpy/srfit/util/argbinders.py +++ b/src/diffpy/srfit/util/argbinders.py @@ -18,11 +18,20 @@ class bind2nd(object): - """Freeze second argument of a callable object to a given - constant.""" + """Freeze the second argument of a callable to a constant. + + Calling the instance as ``bound(a, *args, **kwargs)`` calls + ``func(a, arg1, *args, **kwargs)``. + + Parameters + ---------- + func : callable + The callable whose second argument is frozen. + arg1 + The value passed as the second argument of ``func``. + """ def __init__(self, func, arg1): - """Freeze the second argument of function func to arg1.""" self.func = func self.arg1 = arg1 return diff --git a/src/diffpy/srfit/util/inpututils.py b/src/diffpy/srfit/util/inpututils.py index 1e0d83fa..a2684638 100644 --- a/src/diffpy/srfit/util/inpututils.py +++ b/src/diffpy/srfit/util/inpututils.py @@ -14,28 +14,43 @@ ############################################################################## """Input utilities.""" -__all__ = ["inputToString"] +__all__ = ["convert_input_to_string", "inputToString"] import os.path from pathlib import Path +from diffpy.utils._deprecator import build_deprecation_message, deprecated -def inputToString(input): - """Convert input from various modes to a string. +inputToString_dep_msg = build_deprecation_message( + "diffpy.srfit.util.inpututils", + "inputToString", + "convert_input_to_string", + "4.0.0", +) + + +def convert_input_to_string(input): + """Return the contents of an open file, a file name, or a string. - This is useful when you want a method to accept a string, open file object - or file name. + This lets a method accept its input in any of those forms. A string + that is an existing file, or that is a single line shorter than 80 + characters, is treated as a file name. Any other string is returned + unchanged. Parameters ---------- - input - An open file-like object, name of a file - or a string containing the input. + input : file-like or str + The open file-like object, the name of a file, or the text itself. + Returns + ------- + str + The contents of the input as a string. - Returns the input in a string - Raises IOError if the input is supected to be a file name, but the file - cannot be found. + Raises + ------ + FileNotFoundError + If ``input`` looks like a file name but the file does not exist. """ # Get the input into a string inptstr = "" @@ -52,6 +67,17 @@ def inputToString(input): return inptstr +@deprecated(inputToString_dep_msg) +def inputToString(input): + """This function has been deprecated and will be removed in version + 4.0.0. + + Please use diffpy.srfit.util.inpututils.convert_input_to_string + instead. + """ + return convert_input_to_string(input) + + def get_dict_from_results_file( results_filepath: Path | str, ) -> dict[str, float]: diff --git a/src/diffpy/srfit/util/nameutils.py b/src/diffpy/srfit/util/nameutils.py index e11820be..098292d1 100644 --- a/src/diffpy/srfit/util/nameutils.py +++ b/src/diffpy/srfit/util/nameutils.py @@ -14,32 +14,83 @@ ############################################################################## """Name utilities.""" -__all__ = ["isIdentifier", "validateName"] +__all__ = ["is_identifier", "validate_name", "isIdentifier", "validateName"] import re +from diffpy.utils._deprecator import build_deprecation_message, deprecated + reident = re.compile(r"^[a-zA-Z_]\w*$") +nameutils_base = "diffpy.srfit.util.nameutils" +removal_version = "4.0.0" +isIdentifier_dep_msg = build_deprecation_message( + nameutils_base, "isIdentifier", "is_identifier", removal_version +) +validateName_dep_msg = build_deprecation_message( + nameutils_base, "validateName", "validate_name", removal_version +) + + +def is_identifier(name): + """Check whether a string is a valid name. + + A valid name starts with a letter or underscore and contains only + letters, digits and underscores. + + Parameters + ---------- + name : str + The string to check. + + Returns + ------- + bool + The flag that is True if ``name`` is a valid name. + """ + return reident.match(name) is not None + +def validate_name(name): + """Raise an error if a string is not a valid name. + + Parameters + ---------- + name : str + The string to check. + + Raises + ------ + ValueError + If ``name`` is not a valid name, as defined by ``is_identifier``. + """ + if not is_identifier(name): + raise ValueError( + f"Name '{name}' is not a valid identifier. " + "Please use only letters, digits and underscores, " + "and do not start the name with a digit." + ) + return + + +@deprecated(isIdentifier_dep_msg) def isIdentifier(s): - """Check to see if a python string is a valid identifier. + """This function has been deprecated and will be removed in version + 4.0.0. - From http://code.activestate.com/recipes/413487/ + Please use diffpy.srfit.util.nameutils.is_identifier instead. """ - if reident.match(s) is None: - return False - return True + return is_identifier(s) +@deprecated(validateName_dep_msg) def validateName(name): - """Validate that a name is a valid identifier. + """This function has been deprecated and will be removed in version + 4.0.0. - Raises ValueError if the name is invalid. + Please use diffpy.srfit.util.nameutils.validate_name instead. """ - # Check that the name is valid - if not isIdentifier(name): - raise ValueError(f"Name {name} is not a valid identifier") - return + return validate_name(name) # End of file diff --git a/src/diffpy/srfit/util/observable.py b/src/diffpy/srfit/util/observable.py index 08f2f33b..e39ea592 100644 --- a/src/diffpy/srfit/util/observable.py +++ b/src/diffpy/srfit/util/observable.py @@ -15,6 +15,19 @@ from diffpy.srfit.util.weakrefcallable import weak_ref +from diffpy.utils._deprecator import build_deprecation_message, deprecated + +observable_base = "diffpy.srfit.util.observable.Observable" +removal_version = "4.0.0" +addObserver_dep_msg = build_deprecation_message( + observable_base, "addObserver", "add_observer", removal_version +) +removeObserver_dep_msg = build_deprecation_message( + observable_base, "removeObserver", "remove_observer", removal_version +) +hasObserver_dep_msg = build_deprecation_message( + observable_base, "hasObserver", "has_observer", removal_version +) class Observable(object): @@ -29,18 +42,21 @@ class Observable(object): value depends on them so that the dependents can be notified about value changes and forced to recompute their own value. - The event handlers are callables that take the observable instance - as their single argument. - - interface: - addObserver: registers its callable argument with the list of - handlers to invoke removeObserver: remove an event handler from - the list of handlers to invoke notify: invoke the registered - handlers in the order in which they were registered + The event handlers are callables that take a tuple starting with the + observable instance as their single argument. Register them with + ``add_observer``, remove them with ``remove_observer``, and invoke + them all with ``notify``. """ def notify(self, other=()): - """Notify all observers.""" + """Notify all observers. + + Parameters + ---------- + other : tuple, optional + The extra objects passed to each observer after this + observable. Default is an empty tuple. + """ # build a list before notification, just in case the observer's # callback behavior involves removing itself from our callback set semaphores = (self,) + other @@ -50,24 +66,85 @@ def notify(self, other=()): # callback management - def addObserver(self, callable): - """Add callable to the set of observers.""" + def add_observer(self, callable): + """Add a callable to the set of observers. + + Bound methods are held by weak reference, and are removed + automatically once their object is deleted. + + Parameters + ---------- + callable : callable + The observer to call when this observable notifies. + """ f = weak_ref(callable, fallback=_fb_remove_observer) self._observers.add(f) return - def removeObserver(self, callable): - """Remove callable from the set of observers.""" + def remove_observer(self, callable): + """Remove a callable from the set of observers. + + Parameters + ---------- + callable : callable + The observer to remove. + + Raises + ------ + KeyError + If ``callable`` is not an observer of this observable. + """ f = weak_ref(callable) self._observers.remove(f) return - def hasObserver(self, callable): - """True if `callable` is present in the set of observers.""" + def has_observer(self, callable): + """Check whether a callable is in the set of observers. + + Parameters + ---------- + callable : callable + The observer to look for. + + Returns + ------- + bool + The flag that is True if ``callable`` is an observer. + """ f = weak_ref(callable) rv = f in self._observers return rv + @deprecated(addObserver_dep_msg) + def addObserver(self, callable): + """This function has been deprecated and will be removed in + version 4.0.0. + + Please use diffpy.srfit.util.observable.Observable.add_observer + instead. + """ + return self.add_observer(callable) + + @deprecated(removeObserver_dep_msg) + def removeObserver(self, callable): + """This function has been deprecated and will be removed in + version 4.0.0. + + Please use diffpy.srfit.util.observable.Observable.remove_observer + instead. + """ + return self.remove_observer(callable) + + @deprecated(hasObserver_dep_msg) + def hasObserver(self, callable): + """This function has been deprecated and will be removed in + version 4.0.0. + + Please use diffpy.srfit.util.observable.Observable.has_observer + instead. + """ + return self.has_observer(callable) + # meta methods def __init__(self, **kwds): @@ -86,7 +163,7 @@ def _fb_remove_observer(fobs, semaphores): # This is called from Observable.notify when the WeakBoundMethod # associated object dies. observable = semaphores[0] - observable.removeObserver(fobs) + observable.remove_observer(fobs) return diff --git a/src/diffpy/srfit/util/tagmanager.py b/src/diffpy/srfit/util/tagmanager.py index 37c58a3d..537eaa20 100644 --- a/src/diffpy/srfit/util/tagmanager.py +++ b/src/diffpy/srfit/util/tagmanager.py @@ -22,6 +22,17 @@ import functools +from diffpy.utils._deprecator import build_deprecation_message, deprecated + +tagmanager_base = "diffpy.srfit.util.tagmanager.TagManager" +removal_version = "4.0.0" +hasTags_dep_msg = build_deprecation_message( + tagmanager_base, "hasTags", "has_tags", removal_version +) +verifyTags_dep_msg = build_deprecation_message( + tagmanager_base, "verifyTags", "verify_tags", removal_version +) + class TagManager(object): """TagManager class. @@ -30,22 +41,26 @@ class TagManager(object): Attributes ---------- - silent - Flag indicating whether to silently pass by when a tag - cannot be found (bool, True). If this is False, then a - KeyError will be thrown when a tag cannot be found. - _tagdict - A dictionary of tags to sets of tagged objects. + silent : bool + The flag that is True (default) to treat an unknown tag as having + no objects. If False, an unknown tag raises a KeyError. + _tagdict : dict + The mapping of each tag to the set of objects it is applied to. """ def __init__(self): - """Initialization.""" self._tagdict = {} self.silent = True return def alltags(self): - """Get all tags managed by the TagManager.""" + """Return all tags managed by the TagManager. + + Returns + ------- + dict_keys + The tags that have been applied to any object. + """ return self._tagdict.keys() def tag(self, obj, *tags): @@ -55,13 +70,15 @@ def tag(self, obj, *tags): Parameters ---------- - obj - Any hashable object to be untagged. + obj : hashable + The object to tag. *tags - Tags to apply to obj. - + The tags to apply to ``obj``. - Raises TypeError if obj is not hashable. + Raises + ------ + TypeError + If ``obj`` is not hashable. """ for tag in tags: oset = self._tagdict.setdefault(str(tag), set()) @@ -73,15 +90,17 @@ def untag(self, obj, *tags): Parameters ---------- - obj - Any hashable object to be untagged. + obj : hashable + The object to untag. *tags - Tags to remove from obj. If this is empty, then all - tags will be removed from obj. - - - Raises KeyError if a passed tag does not apply to obj and self.silent - is False + The tags to remove from ``obj``. If none are given, all tags + are removed from ``obj``. + + Raises + ------ + KeyError + If a given tag does not apply to ``obj`` and ``silent`` is + False. """ if not tags: tags = self.tags(obj) @@ -95,26 +114,63 @@ def untag(self, obj, *tags): return def tags(self, obj): - """Get all tags on an object. + """Return all tags on an object. - Returns list + Parameters + ---------- + obj : hashable + The object to look up. + + Returns + ------- + list of str + The tags applied to ``obj``. """ tags = [k for (k, v) in self._tagdict.items() if obj in v] return tags - def hasTags(self, obj, *tags): - """Determine if an object has all passed tags. + def has_tags(self, obj, *tags): + """Check whether an object has all of the given tags. - Returns bool + Parameters + ---------- + obj : hashable + The object to check. + *tags + The tags to look for. + + Returns + ------- + bool + The flag that is True if ``obj`` has every tag in ``tags``. + + Raises + ------ + KeyError + If a tag does not exist and ``silent`` is False. """ setgen = (self.__get_object_set(t) for t in tags) result = all(obj in s for s in setgen) return result def union(self, *tags): - """Get all objects that have any of the passed tags. + """Return all objects that have any of the given tags. - Returns set + Parameters + ---------- + *tags + The tags to look for. + + Returns + ------- + set + The objects that have at least one tag in ``tags``. Empty if + no tags are given. + + Raises + ------ + KeyError + If a tag does not exist and ``silent`` is False. """ if not tags: return set() @@ -123,9 +179,23 @@ def union(self, *tags): return objs def intersection(self, *tags): - """Get all objects that have all of the passed tags. + """Return all objects that have all of the given tags. - Returns set + Parameters + ---------- + *tags + The tags to look for. + + Returns + ------- + set + The objects that have every tag in ``tags``. Empty if no tags + are given. + + Raises + ------ + KeyError + If a tag does not exist and ``silent`` is False. """ if not tags: return set() @@ -133,11 +203,25 @@ def intersection(self, *tags): objs = functools.reduce(set.intersection, setgen) return objs - def verifyTags(self, *tags): - """Check that tags are all extant. + def verify_tags(self, *tags): + """Check that all of the given tags exist. + + This ignores ``silent``. + + Parameters + ---------- + *tags + The tags to check. - Raises KeyError if a passed tag does not exist. This ignores - self.silent. + Returns + ------- + bool + True when every tag exists. + + Raises + ------ + KeyError + If a tag does not exist. """ keys = self._tagdict.keys() for tag in tags: @@ -145,6 +229,26 @@ def verifyTags(self, *tags): raise KeyError("Tag '%s' does not exist" % tag) return True + @deprecated(hasTags_dep_msg) + def hasTags(self, obj, *tags): + """This function has been deprecated and will be removed in + version 4.0.0. + + Please use diffpy.srfit.util.tagmanager.TagManager.has_tags + instead. + """ + return self.has_tags(obj, *tags) + + @deprecated(verifyTags_dep_msg) + def verifyTags(self, *tags): + """This function has been deprecated and will be removed in + version 4.0.0. + + Please use diffpy.srfit.util.tagmanager.TagManager.verify_tags + instead. + """ + return self.verify_tags(*tags) + def __get_object_set(self, tag): """Helper function for getting an object set with given tag. diff --git a/src/diffpy/srfit/util/weakrefcallable.py b/src/diffpy/srfit/util/weakrefcallable.py index 9a30ac72..c26aa4cc 100644 --- a/src/diffpy/srfit/util/weakrefcallable.py +++ b/src/diffpy/srfit/util/weakrefcallable.py @@ -21,39 +21,37 @@ class WeakBoundMethod(object): """Callable wrapper to a bound method stored as a weak reference. - Support storage of bound methods without keeping the associated objects - alive forever. Provide facility for a fallback function to be used - when method-related object is deleted. + This stores a bound method without keeping its object alive, and can + call a fallback function once that object has been deleted. Attributes ---------- - function : MethodType - the unbound function extracted from the wrapped bound method. - fallback : FunctionType, optional - plain function to be called when object holding the bound method - gets deallocated. The fallback function is called with this - object as the first argument followed by any positional and - keyword arguments passed for the bound method. The fallback - function can be used to deregister this wrapper. - _wref : weakref - weak reference to the object the wrapped method is bound to. + function : FunctionType + The unbound function extracted from the wrapped bound method. + fallback : FunctionType or None + The plain function called when the object holding the bound method + has been deleted. It is called with this wrapper as the first + argument, followed by any positional and keyword arguments passed + for the bound method, and can be used to deregister this wrapper. + _wref : weakref.ref + The weak reference to the object the wrapped method is bound to. _class : type - the type of the object to which the method is bound. - This is only used for pickling. + The type of the object the method is bound to. This is only used + for pickling. """ __slots__ = ("function", "fallback", "_wref", "_class") def __init__(self, f, fallback=None): - """Create a weak reference wrapper to bound method. + """Create a weak reference wrapper to a bound method. Parameters ---------- - f : bound MethodType - instance-bound method to be wrapped. + f : MethodType + The instance-bound method to wrap. fallback : FunctionType, optional - plain function to be called instead of the bound method when - the method associated object gets deallocated. + The plain function called instead of the bound method once the + method's object has been deleted. Default is None. """ # This does not handle builtin methods, but that can be added # if necessary. @@ -64,22 +62,27 @@ def __init__(self, f, fallback=None): return def __call__(self, *args, **kwargs): - """Call the wrapped method if the weak-referenced object is - alive. + """Call the wrapped method if its object is still alive. - If that object does not exist and the fallback function is defined, + If that object has been deleted and a fallback function is set, call the fallback function instead. Parameters ---------- *args, **kwargs - same arguments as for the wrapped bound method. + The arguments passed to the wrapped bound method. + + Returns + ------- + object + The return value of the bound method, or of the fallback + function once the object has been deleted. Raises ------ ReferenceError - when the method-bound object does not exist and the fallback - function is not defined. + If the method's object has been deleted and no fallback + function is set. """ mobj = self._wref() if mobj is not None: @@ -141,20 +144,20 @@ def weak_ref(f, fallback=None): Parameters ---------- f : callable - object-bound method or a plain function. + The object-bound method or plain function to wrap. fallback : FunctionType, optional - plain function to be called when object holding ``f`` gets - deallocated. The fallback function is called with the - wrapper object as the first argument followed by positional - and keyword arguments passed for the bound method. The - fallback function can be used to deregister the wrapper. + The plain function called when the object holding ``f`` has been + deleted. It is called with the wrapper as the first argument, + followed by the positional and keyword arguments passed for the + bound method, and can be used to deregister the wrapper. Default + is None. Returns ------- - WeakBoundMethod - when `f` is a bound method. If `f` is a plain function or - already of WeakBoundMethod type, return `f` and ignore the - `fallback` argument. + WeakBoundMethod or callable + The weak wrapper when ``f`` is a bound method. When ``f`` is a + plain function or already a WeakBoundMethod, ``f`` itself, and + ``fallback`` is ignored. """ # NOTE Weak referencing plain functions is probably not needed, # because they are already bound to the defining modules. diff --git a/tests/conftest.py b/tests/conftest.py index 45687a21..d19f976e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -453,3 +453,14 @@ def reset_characteristic_function_warnings(): _warned_non_physical.clear() yield _warned_non_physical.clear() + + +@pytest.fixture +def tag_manager(): + from diffpy.srfit.util.tagmanager import TagManager + + manager = TagManager() + manager.silent = False + manager.tag(3, "3", "number") + manager.tag(4, "4", "number") + return manager diff --git a/tests/deprecated/__init__.py b/tests/deprecated/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/deprecated/test_util_deprecations.py b/tests/deprecated/test_util_deprecations.py new file mode 100644 index 00000000..08eca3f2 --- /dev/null +++ b/tests/deprecated/test_util_deprecations.py @@ -0,0 +1,302 @@ +"""Tests for the deprecated camelCase names in diffpy.srfit.util. + +Each test repeats the cases of the matching test for the new name, but +calls the deprecated name and also checks that it warns. + +Remove this file in version 4.0.0, together with the deprecated names. +""" + +import io +import re + +import pytest + +from diffpy.srfit.util import sortKeyForNumericString +from diffpy.srfit.util.inpututils import inputToString +from diffpy.srfit.util.nameutils import isIdentifier, validateName +from diffpy.srfit.util.observable import Observable + +MULTILINE_TEXT = "a 1.0\nb 2.0\n" +LONG_TEXT = "x" * 80 + + +def expected_message(base, old_name, new_name): + return re.escape( + f"'{base}.{old_name}' is deprecated and will be removed in version " + f"4.0.0. Please use '{base}.{new_name}' instead." + ) + + +OBSERVABLE = "diffpy.srfit.util.observable.Observable" +TAG_MANAGER = "diffpy.srfit.util.tagmanager.TagManager" +NAMEUTILS = "diffpy.srfit.util.nameutils" + + +class _Recorder: + """Observer that records the arguments it is notified with.""" + + def __init__(self): + self.calls = [] + + def record(self, semaphores): + self.calls.append(semaphores) + + +# Observable: see test_observable.py::test_add_and_remove_observer. +def test_addObserver_hasObserver_removeObserver(): + # C1: An observer is added, then removed. + # Expected: Each call warns. hasObserver is True after adding and False + # after removing. + observable = Observable() + recorder = _Recorder() + with pytest.warns( + DeprecationWarning, + match=expected_message(OBSERVABLE, "addObserver", "add_observer"), + ): + observable.addObserver(recorder.record) + with pytest.warns( + DeprecationWarning, + match=expected_message(OBSERVABLE, "hasObserver", "has_observer"), + ): + actual_has_observer_after_add = observable.hasObserver(recorder.record) + expected_has_observer_after_add = True + assert actual_has_observer_after_add == expected_has_observer_after_add + with pytest.warns( + DeprecationWarning, + match=expected_message( + OBSERVABLE, "removeObserver", "remove_observer" + ), + ): + observable.removeObserver(recorder.record) + actual_has_observer_after_remove = observable.has_observer(recorder.record) + expected_has_observer_after_remove = False + assert ( + actual_has_observer_after_remove == expected_has_observer_after_remove + ) + + +# TagManager: see test_tagmanager.py::test_has_tags. +@pytest.mark.parametrize( + "input_tags, input_silent, expected_has_tags", + [ + # C1: The object has the one given tag. + # Expected: Warns and returns True. + (("3",), False, True), + # C2: The object has both given tags. + # Expected: Warns and returns True. + (("3", "number"), False, True), + # C3: The object has only one of the given tags. + # Expected: Warns and returns False. + (("3", "4"), False, False), + # C4: A silent manager is asked about an unknown tag. + # Expected: Warns and returns False. + (("fail",), True, False), + ], +) +def test_hasTags(tag_manager, input_tags, input_silent, expected_has_tags): + tag_manager.silent = input_silent + with pytest.warns( + DeprecationWarning, + match=expected_message(TAG_MANAGER, "hasTags", "has_tags"), + ): + actual_has_tags = tag_manager.hasTags(3, *input_tags) + assert actual_has_tags == expected_has_tags + + +# TagManager: see test_tagmanager.py::test_has_tags_unknown_tag_bad. +def test_hasTags_unknown_tag_bad(tag_manager): + # C1: hasTags(3, "fail") is called, but no object was ever tagged + # "fail", and the manager is not silent. + # Expected: Warns and raises a KeyError. + with ( + pytest.warns( + DeprecationWarning, + match=expected_message(TAG_MANAGER, "hasTags", "has_tags"), + ), + pytest.raises(KeyError), + ): + tag_manager.hasTags(3, "fail") + + +# TagManager: see test_tagmanager.py::test_verify_tags. +@pytest.mark.parametrize( + "input_tags, input_silent, expected_error", + [ + # C1: All tags exist. + # Expected: Warns and returns True. + (("3", "number"), False, None), + # C2: One tag does not exist and the manager is not silent. + # Expected: Warns and raises a KeyError. + (("3", "fail"), False, KeyError), + # C3: One tag does not exist and the manager is silent. + # Expected: Warns and still raises a KeyError. + (("3", "fail"), True, KeyError), + ], +) +def test_verifyTags(tag_manager, input_tags, input_silent, expected_error): + tag_manager.silent = input_silent + expected_warning_message = expected_message( + TAG_MANAGER, "verifyTags", "verify_tags" + ) + if expected_error is None: + with pytest.warns(DeprecationWarning, match=expected_warning_message): + actual_result = tag_manager.verifyTags(*input_tags) + expected_result = True + assert actual_result == expected_result + else: + with ( + pytest.warns(DeprecationWarning, match=expected_warning_message), + pytest.raises(expected_error), + ): + tag_manager.verifyTags(*input_tags) + + +# nameutils: see test_nameutils.py::test_is_identifier. +@pytest.mark.parametrize( + "input_name, expected_is_identifier", + [ + # C1: A single letter. + # Expected: Warns and returns True. + ("x", True), + # C2: Letters, digits and underscores, starting with an underscore. + # Expected: Warns and returns True. + ("_scale_2", True), + # C3: The name starts with a digit. + # Expected: Warns and returns False. + ("2x", False), + # C4: The name contains a hyphen. + # Expected: Warns and returns False. + ("a-b", False), + # C5: The name contains a space. + # Expected: Warns and returns False. + ("a b", False), + # C6: The name is empty. + # Expected: Warns and returns False. + ("", False), + ], +) +def test_isIdentifier(input_name, expected_is_identifier): + with pytest.warns( + DeprecationWarning, + match=expected_message(NAMEUTILS, "isIdentifier", "is_identifier"), + ): + actual_is_identifier = isIdentifier(input_name) + assert actual_is_identifier == expected_is_identifier + + +# nameutils: see test_nameutils.py::test_validate_name_valid. +@pytest.mark.parametrize( + "input_name", + [ + # C1: A valid name. + # Expected: Warns and raises no error. + "scale", + # C2: A valid name with digits and underscores. + # Expected: Warns and raises no error. + "_scale_2", + ], +) +def test_validateName_valid(input_name): + with pytest.warns( + DeprecationWarning, + match=expected_message(NAMEUTILS, "validateName", "validate_name"), + ): + actual_result = validateName(input_name) + expected_result = None + assert actual_result == expected_result + + +# nameutils: see test_nameutils.py::test_validate_name_invalid. +@pytest.mark.parametrize( + "input_name", + [ + # C1: The name starts with a digit. + # Expected: Warns and raises a ValueError that explains the fix. + "2x", + # C2: The name contains a hyphen. + # Expected: Warns and raises a ValueError that explains the fix. + "a-b", + ], +) +def test_validateName_invalid(input_name): + expected_error_message = ( + f"Name '{input_name}' is not a valid identifier. " + "Please use only letters, digits and underscores, " + "and do not start the name with a digit." + ) + with ( + pytest.warns( + DeprecationWarning, + match=expected_message(NAMEUTILS, "validateName", "validate_name"), + ), + pytest.raises(ValueError, match=re.escape(expected_error_message)), + ): + validateName(input_name) + + +# inpututils: see test_inpututils.py::test_convert_input_to_string. +@pytest.mark.parametrize( + "input_kind, expected_string", + [ + # C1: An open file-like object. + # Expected: Warns and returns its contents. + ("file_object", MULTILINE_TEXT), + # C2: The name of an existing file. + # Expected: Warns and returns the file's contents. + ("file_name", MULTILINE_TEXT), + # C3: Text spanning several lines. + # Expected: Warns and returns the text itself. + ("multiline_text", MULTILINE_TEXT), + # C4: A single line of 80 or more characters. + # Expected: Warns and returns the text itself. + ("long_text", LONG_TEXT), + ], +) +def test_inputToString(tmp_path, input_kind, expected_string): + file_path = tmp_path / "input.txt" + file_path.write_text(MULTILINE_TEXT) + inputs = { + "file_object": io.StringIO(MULTILINE_TEXT), + "file_name": str(file_path), + "multiline_text": MULTILINE_TEXT, + "long_text": LONG_TEXT, + } + with pytest.warns( + DeprecationWarning, + match=expected_message( + "diffpy.srfit.util.inpututils", + "inputToString", + "convert_input_to_string", + ), + ): + actual_string = inputToString(inputs[input_kind]) + assert actual_string == expected_string + + +# util: see test_util.py::test_sort_key_for_numeric_string. +@pytest.mark.parametrize( + "input_string, expected_key", + [ + # C1: Text, a number, then more text. + # Expected: Warns, and the number becomes an integer between the + # text parts. + ("a12b", ("a", 12, "b")), + # C2: The string starts with a number. + # Expected: Warns, and the key starts with an empty text part. + ("12a", ("", 12, "a")), + # C3: The string has no digits. + # Expected: Warns, and the key holds the string unchanged. + ("abc", ("abc",)), + ], +) +def test_sortKeyForNumericString(input_string, expected_key): + with pytest.warns( + DeprecationWarning, + match=expected_message( + "diffpy.srfit.util", + "sortKeyForNumericString", + "sort_key_for_numeric_string", + ), + ): + actual_key = sortKeyForNumericString(input_string) + assert actual_key == expected_key diff --git a/tests/test_inpututils.py b/tests/test_inpututils.py new file mode 100644 index 00000000..c50ae4ba --- /dev/null +++ b/tests/test_inpututils.py @@ -0,0 +1,70 @@ +"""Unit tests for inpututils.py.""" + +import io + +import pytest + +from diffpy.srfit.util.inpututils import ( + convert_input_to_string, + get_dict_from_results_file, +) + +MULTILINE_TEXT = "a 1.0\nb 2.0\n" +LONG_TEXT = "x" * 80 + + +# The input may be an open file, a file name, or the text itself. +@pytest.mark.parametrize( + "input_kind, expected_string", + [ + # C1: An open file-like object. + # Expected: Its contents. + ("file_object", MULTILINE_TEXT), + # C2: The name of an existing file. + # Expected: The file's contents. + ("file_name", MULTILINE_TEXT), + # C3: Text spanning several lines. + # Expected: The text itself. + ("multiline_text", MULTILINE_TEXT), + # C4: A single line of 80 or more characters. + # Expected: The text itself. + ("long_text", LONG_TEXT), + ], +) +def test_convert_input_to_string(tmp_path, input_kind, expected_string): + file_path = tmp_path / "input.txt" + file_path.write_text(MULTILINE_TEXT) + inputs = { + "file_object": io.StringIO(MULTILINE_TEXT), + "file_name": str(file_path), + "multiline_text": MULTILINE_TEXT, + "long_text": LONG_TEXT, + } + actual_string = convert_input_to_string(inputs[input_kind]) + assert actual_string == expected_string + + +def test_convert_input_to_string_missing_file(): + # C1: A short single line that is not an existing file. + # Expected: It is treated as a file name, so FileNotFoundError is raised. + with pytest.raises(FileNotFoundError): + convert_input_to_string("missing_file.txt") + + +def test_get_dict_from_results_file(tmp_path): + # C1: A results file with parameter lines, a separator, a blank line, + # a line without an uncertainty and a line with a non-numeric value. + # Expected: Only well-formed "name value +/- uncertainty" lines are read. + results_file = tmp_path / "results.res" + results_file.write_text( + "Variables\n" + "--------------\n" + "a 1.5 +/- 0.1\n" + "\n" + "b 2 +/- 0.3\n" + "c 4.0\n" + "d value +/- 0.1\n" + ) + actual_results = get_dict_from_results_file(results_file) + expected_results = {"a": 1.5, "b": 2.0} + assert actual_results == expected_results diff --git a/tests/test_nameutils.py b/tests/test_nameutils.py new file mode 100644 index 00000000..033af174 --- /dev/null +++ b/tests/test_nameutils.py @@ -0,0 +1,74 @@ +"""Unit tests for nameutils.py.""" + +import re + +import pytest + +from diffpy.srfit.util.nameutils import is_identifier, validate_name + + +# A valid name starts with a letter or underscore and contains only +# letters, digits and underscores. +@pytest.mark.parametrize( + "input_name, expected_is_identifier", + [ + # C1: A single letter. + # Expected: Valid. + ("x", True), + # C2: Letters, digits and underscores, starting with an underscore. + # Expected: Valid. + ("_scale_2", True), + # C3: The name starts with a digit. + # Expected: Not valid. + ("2x", False), + # C4: The name contains a hyphen. + # Expected: Not valid. + ("a-b", False), + # C5: The name contains a space. + # Expected: Not valid. + ("a b", False), + # C6: The name is empty. + # Expected: Not valid. + ("", False), + ], +) +def test_is_identifier(input_name, expected_is_identifier): + actual_is_identifier = is_identifier(input_name) + assert actual_is_identifier == expected_is_identifier + + +@pytest.mark.parametrize( + "input_name", + [ + # C1: A valid name. + # Expected: No error is raised. + "scale", + # C2: A valid name with digits and underscores. + # Expected: No error is raised. + "_scale_2", + ], +) +def test_validate_name_valid(input_name): + actual_result = validate_name(input_name) + assert actual_result is None + + +@pytest.mark.parametrize( + "input_name", + [ + # C1: The name starts with a digit. + # Expected: A ValueError explains the problem and the fix. + "2x", + # C2: The name contains a hyphen. + # Expected: A ValueError explains the problem and the fix. + "a-b", + ], +) +def test_validate_name_invalid(input_name): + expected_message = ( + f"Name '{input_name}' is not a valid identifier. " + "Please use only letters, digits and underscores, " + "and do not start the name with a digit." + ) + with pytest.raises(ValueError, match=re.escape(expected_message)): + validate_name(input_name) diff --git a/tests/test_observable.py b/tests/test_observable.py new file mode 100644 index 00000000..75d78c05 --- /dev/null +++ b/tests/test_observable.py @@ -0,0 +1,42 @@ +"""Unit tests for observable.py.""" + +from diffpy.srfit.util.observable import Observable + + +class _Recorder: + """Observer that records the arguments it is notified with.""" + + def __init__(self): + self.calls = [] + + def record(self, semaphores): + self.calls.append(semaphores) + + +def test_add_and_remove_observer(): + # C1: An observer is added, then removed. + # Expected: has_observer is True after adding and False after removing. + observable = Observable() + recorder = _Recorder() + assert observable.has_observer(recorder.record) is False + observable.add_observer(recorder.record) + assert observable.has_observer(recorder.record) is True + observable.remove_observer(recorder.record) + actual_has_observer = observable.has_observer(recorder.record) + expected_has_observer = False + assert actual_has_observer == expected_has_observer + + +def test_notify(): + # C1: Two observers are added and the observable notifies with an extra + # object. + # Expected: Each observer is called once with the observable followed + # by the extra object. + observable = Observable() + first, second = _Recorder(), _Recorder() + observable.add_observer(first.record) + observable.add_observer(second.record) + observable.notify(("extra",)) + expected_calls = [(observable, "extra")] + assert first.calls == expected_calls + assert second.calls == expected_calls diff --git a/tests/test_tagmanager.py b/tests/test_tagmanager.py index e340e2a5..da3551d0 100644 --- a/tests/test_tagmanager.py +++ b/tests/test_tagmanager.py @@ -14,99 +14,193 @@ ############################################################################## """Unit tests for tagmanager.py.""" -import unittest - -from diffpy.srfit.util.tagmanager import TagManager - - -############################################################################## -class TestTagManager(unittest.TestCase): - - def setUp(self): - self.m = TagManager() - self.m.silent = False - return - - def tearDown(self): - return - - def test_tag(self): - """Check TagManager.tag()""" - m = self.m - obj = 3 - m.tag(obj, "3", "three") - m.tag(obj, "tri") - tags = set(m.tags(obj)) - self.assertEqual(set(["3", "three", "tri"]), tags) - - # Try an unhashable object - obj = set() - self.assertRaises(TypeError, obj, "unhashable") - - return - - def test_untag(self): - """Check TagManager.untag()""" - m = self.m - obj = 3 - m.tag(obj, "3", "three", "tri", "tres", "trois") - tags = set(m.tags(obj)) - self.assertEqual(set(["3", "three", "tri", "tres", "trois"]), tags) - self.assertRaises(KeyError, m.untag, obj, "4") - self.assertRaises(KeyError, m.untag, 4, "3") - self.assertRaises(KeyError, m.untag, 4, "4") - m.untag(obj, "3") - tags = set(m.tags(obj)) - self.assertEqual(set(["three", "tri", "tres", "trois"]), tags) - m.untag(obj, "three", "tri") - tags = set(m.tags(obj)) - self.assertEqual(set(["tres", "trois"]), tags) - m.untag(obj) - tags = set(m.tags(obj)) - self.assertEqual(set(), tags) - return - - def test_union_and_intersection(self): - """Check TagManager.union() and TagManager.intersection()""" - m = self.m - m.tag(3, "3", "number") - m.tag(4, "4", "number") - objs = set([3, 4]) - self.assertEqual(m.union(), set()) - self.assertEqual(m.union("number"), objs) - self.assertEqual(m.union("3"), set([3])) - self.assertEqual(m.union("3", "4"), objs) - self.assertRaises(KeyError, m.union, "fail") - m.silent = True - self.assertEqual(set(), m.union("fail")) - self.assertEqual(set([3]), m.union("fail", "3")) - m.silent = False - self.assertEqual(m.intersection(), set()) - self.assertEqual(m.intersection("number"), objs) - self.assertEqual(m.intersection("3"), set([3])) - self.assertEqual(m.intersection("3", "4"), set()) - self.assertRaises(KeyError, m.intersection, "fail") - m.silent = True - self.assertEqual(set(), m.intersection("fail")) - return - - def test_hasTags(self): - """Check TagManager.hasTags()""" - m = self.m - m.tag(3, "3", "number") - m.tag(4, "4", "number") - self.assertTrue(m.hasTags(3, "3")) - self.assertTrue(m.hasTags(3, "3", "number")) - self.assertFalse(m.hasTags(3, "3", "4")) - self.assertRaises(KeyError, m.hasTags, 3, "fail") - m.silent = True - self.assertFalse(m.hasTags(3, "fail")) - return - - -# End of class TestTagManager - -if __name__ == "__main__": - unittest.main() - -# End of file +import pytest + +# The tag_manager fixture (conftest.py) tags 3 with "3" and "number", and 4 +# with "4" and "number". It is not silent unless a test makes it so. + + +# Tagging stores every tag, as a string, on the object. +@pytest.mark.parametrize( + "input_tag_calls, expected_tags", + [ + # C1: A new object is tagged once with one tag. + # Expected: The object has that tag. + ([("three",)], {"three"}), + # C2: A new object is tagged in two calls with several tags. + # Expected: The object has every tag from both calls. + ([("three", "tri"), ("trois",)], {"three", "tri", "trois"}), + # C3: A new object is tagged with a non-string tag. + # Expected: The tag is stored as a string. + ([(3,)], {"3"}), + ], +) +def test_tag(tag_manager, input_tag_calls, expected_tags): + for tags in input_tag_calls: + tag_manager.tag("obj", *tags) + actual_tags = set(tag_manager.tags("obj")) + assert actual_tags == expected_tags + + +def test_tag_unhashable_object_bad(tag_manager): + # C1: An unhashable object (a set) is tagged. + # Expected: A TypeError is raised. + with pytest.raises(TypeError): + tag_manager.tag(set(), "unhashable") + + +# Untagging removes the given tags, or all tags when none are given. +@pytest.mark.parametrize( + "input_tags, expected_tags", + [ + # C1: One tag is removed. + # Expected: The other tags remain. + (("3",), {"three", "tri", "tres", "trois"}), + # C2: Several tags are removed at once. + # Expected: Only the tags that were not removed remain. + (("3", "three", "tri"), {"tres", "trois"}), + # C3: No tags are given. + # Expected: Every tag is removed from the object. + ((), set()), + ], +) +def test_untag(tag_manager, input_tags, expected_tags): + tag_manager.tag("obj", "3", "three", "tri", "tres", "trois") + tag_manager.untag("obj", *input_tags) + actual_tags = set(tag_manager.tags("obj")) + assert actual_tags == expected_tags + + +# When not silent, untagging something that is not tagged raises KeyError. +@pytest.mark.parametrize( + "input_object, input_tag", + [ + # C1: The tag does not exist. + # Expected: A KeyError is raised. + (3, "5"), + # C2: The tag exists but does not apply to the object. + # Expected: A KeyError is raised. + (4, "3"), + # C3: Neither the tag nor the object is known. + # Expected: A KeyError is raised. + (5, "5"), + ], +) +def test_untag_missing_tag_bad(tag_manager, input_object, input_tag): + with pytest.raises(KeyError): + tag_manager.untag(input_object, input_tag) + + +# Union returns objects with any of the tags; intersection returns objects +# with all of them. A silent manager treats unknown tags as empty. +@pytest.mark.parametrize( + "input_method, input_tags, input_silent, expected_objects", + [ + # C1: Union of no tags. + # Expected: An empty set. + ("union", (), False, set()), + # C2: Union of a tag shared by both objects. + # Expected: Both objects. + ("union", ("number",), False, {3, 4}), + # C3: Union of a tag on one object. + # Expected: That object. + ("union", ("3",), False, {3}), + # C4: Union of tags on different objects. + # Expected: Both objects. + ("union", ("3", "4"), False, {3, 4}), + # C5: Silent union of an unknown tag. + # Expected: An empty set. + ("union", ("fail",), True, set()), + # C6: Silent union of an unknown tag and a known tag. + # Expected: The object with the known tag. + ("union", ("fail", "3"), True, {3}), + # C7: Intersection of no tags. + # Expected: An empty set. + ("intersection", (), False, set()), + # C8: Intersection of a tag shared by both objects. + # Expected: Both objects. + ("intersection", ("number",), False, {3, 4}), + # C9: Intersection of a tag on one object. + # Expected: That object. + ("intersection", ("3",), False, {3}), + # C10: Intersection of tags on different objects. + # Expected: An empty set. + ("intersection", ("3", "4"), False, set()), + # C11: Silent intersection of an unknown tag. + # Expected: An empty set. + ("intersection", ("fail",), True, set()), + ], +) +def test_union_and_intersection( + tag_manager, input_method, input_tags, input_silent, expected_objects +): + tag_manager.silent = input_silent + actual_objects = getattr(tag_manager, input_method)(*input_tags) + assert actual_objects == expected_objects + + +@pytest.mark.parametrize("input_method", ["union", "intersection"]) +def test_union_and_intersection_unknown_tag_bad(tag_manager, input_method): + # C1: union("fail") or intersection("fail") is called, but no object + # was ever tagged "fail", and the manager is not silent. + # Expected: A KeyError is raised. + with pytest.raises(KeyError): + getattr(tag_manager, input_method)("fail") + + +# has_tags is True only when the object has every given tag. +@pytest.mark.parametrize( + "input_tags, input_silent, expected_has_tags", + [ + # C1: The object has the one given tag. + # Expected: True. + (("3",), False, True), + # C2: The object has both given tags. + # Expected: True. + (("3", "number"), False, True), + # C3: The object has only one of the given tags. + # Expected: False. + (("3", "4"), False, False), + # C4: A silent manager is asked about an unknown tag. + # Expected: False. + (("fail",), True, False), + ], +) +def test_has_tags(tag_manager, input_tags, input_silent, expected_has_tags): + tag_manager.silent = input_silent + actual_has_tags = tag_manager.has_tags(3, *input_tags) + assert actual_has_tags == expected_has_tags + + +def test_has_tags_unknown_tag_bad(tag_manager): + # C1: has_tags(3, "fail") is called, but no object was ever tagged + # "fail", and the manager is not silent. + # Expected: A KeyError is raised. + with pytest.raises(KeyError): + tag_manager.has_tags(3, "fail") + + +# verify_tags passes for existing tags and raises KeyError for unknown ones, +# whether or not the manager is silent. +@pytest.mark.parametrize( + "input_tags, input_silent, expected_error", + [ + # C1: All tags exist. + # Expected: Returns True. + (("3", "number"), False, None), + # C2: One tag does not exist and the manager is not silent. + # Expected: A KeyError is raised. + (("3", "fail"), False, KeyError), + # C3: One tag does not exist and the manager is silent. + # Expected: A KeyError is still raised. + (("3", "fail"), True, KeyError), + ], +) +def test_verify_tags(tag_manager, input_tags, input_silent, expected_error): + tag_manager.silent = input_silent + if expected_error is None: + actual_result = tag_manager.verify_tags(*input_tags) + assert actual_result is True + else: + with pytest.raises(expected_error): + tag_manager.verify_tags(*input_tags) diff --git a/tests/test_util.py b/tests/test_util.py new file mode 100644 index 00000000..c1bc6224 --- /dev/null +++ b/tests/test_util.py @@ -0,0 +1,36 @@ +"""Unit tests for the functions in diffpy.srfit.util.""" + +import pytest + +from diffpy.srfit.util import sort_key_for_numeric_string + + +# The key splits a string into text and integer parts so that numbers sort +# by value rather than character by character. +@pytest.mark.parametrize( + "input_string, expected_key", + [ + # C1: Text, a number, then more text. + # Expected: The number becomes an integer between the text parts. + ("a12b", ("a", 12, "b")), + # C2: The string starts with a number. + # Expected: The key starts with an empty text part. + ("12a", ("", 12, "a")), + # C3: The string has no digits. + # Expected: The key holds the string unchanged. + ("abc", ("abc",)), + ], +) +def test_sort_key_for_numeric_string(input_string, expected_key): + actual_key = sort_key_for_numeric_string(input_string) + assert actual_key == expected_key + + +def test_sort_key_for_numeric_string_orders_numbers_by_value(): + # C1: Names whose numbers have different numbers of digits are sorted. + # Expected: "a2" comes before "a10". + actual_order = sorted( + ["a10", "a2", "b1", "a1"], key=sort_key_for_numeric_string + ) + expected_order = ["a1", "a2", "a10", "b1"] + assert actual_order == expected_order diff --git a/tests/test_visitors.py b/tests/test_visitors.py index 49381361..52abaf63 100644 --- a/tests/test_visitors.py +++ b/tests/test_visitors.py @@ -181,8 +181,8 @@ def testSimpleFunction(self): # Check that the operator value is invalidated assert mult._value is None - assert not v2.hasObserver(minus._flush) - assert v5.hasObserver(minus._flush) + assert not v2.has_observer(minus._flush) + assert v5.has_observer(minus._flush) # now get the args args = visitors.getArgs(mult) @@ -200,8 +200,8 @@ def testSimpleFunction(self): plus2 = literals.AdditionOperator() visitors.swap(mult, minus, plus2) assert mult._value is None - assert not minus.hasObserver(mult._flush) - assert plus2.hasObserver(mult._flush) + assert not minus.has_observer(mult._flush) + assert plus2.has_observer(mult._flush) # plus2 has no arguments yet. Verify this. with pytest.raises(TypeError): diff --git a/tests/test_weakrefcallable.py b/tests/test_weakrefcallable.py index 947fc8d8..20c3288d 100644 --- a/tests/test_weakrefcallable.py +++ b/tests/test_weakrefcallable.py @@ -15,132 +15,189 @@ """Unit tests for the weakrefcallable module.""" import pickle -import unittest + +import pytest from diffpy.srfit import FitContribution from diffpy.srfit.fitbase.parameter import Parameter from diffpy.srfit.util.weakrefcallable import WeakBoundMethod, weak_ref -# ---------------------------------------------------------------------------- - - -class TestWeakBoundMethod(unittest.TestCase): - - def setUp(self): - self.f = FitContribution("f") - self.f.set_equation("7") - self.w = weak_ref(self.f._eq._flush, fallback=_fallback_example) - return - - def tearDown(self): - self.f = None - self.assertTrue(None is self.w._wref()) - obj, args, kw = self.w("any", "argument", foo=37) - self.assertTrue(obj is self.w) - self.assertEqual(("any", "argument"), args) - self.assertEqual({"foo": 37}, kw) - return - - def test___init__(self): - """Check WeakBoundMethod.__init__()""" - self.assertTrue(self.w.fallback is _fallback_example) - wf = weak_ref(self.f._flush) - self.assertTrue(None is wf.fallback) - return - - def test___call__(self): - """Check WeakBoundMethod.__call__()""" - f = self.f - self.assertEqual(7, f.evaluate()) - self.assertEqual(7, f._eq._value) - # verify f has the same effect as f._eq._flush - self.w(()) - self.assertTrue(None is f._eq._value) - # check WeakBoundMethod behavior with no fallback - x = Parameter("x", value=3) - wgetx = weak_ref(x.get_value) - self.assertEqual(3, wgetx()) - del x - self.assertRaises(ReferenceError, wgetx) - return - - def test___hash__(self): - """Check WeakBoundMethod.__hash__()""" - f1 = FitContribution("f1") - w1 = weak_ref(f1._flush) - h0 = hash(w1) - del f1 - self.assertTrue(None is w1._wref()) - self.assertEqual(h0, hash(w1)) - w1c1 = pickle.loads(pickle.dumps(w1)) - w1c2 = pickle.loads(pickle.dumps(w1)) - self.assertEqual(hash(w1c1), hash(w1c2)) - return - - def test___eq__(self): - """Check WeakBoundMethod.__eq__()""" - f1 = FitContribution("f1") - w1 = weak_ref(f1._flush) - w2 = weak_ref(f1._flush) - self.assertEqual(w1, w2) - w1c = pickle.loads(pickle.dumps(w1)) - # pickle-copied objects should have empty reference - self.assertTrue(None is w1c._wref()) - self.assertNotEqual(w1, w1c) - del f1 - self.assertTrue(None is w1._wref()) - self.assertEqual(w1, w1c) - w1cc = pickle.loads(pickle.dumps(w1c)) - self.assertTrue(None is w1cc._wref()) - self.assertEqual(w1c, w1cc) - self.assertEqual(w1, w1cc) - return - - def test_pickling(self): - """Verify unpickling works when it involves __hash__ call.""" - holder = set([self.w]) - objs = [holder, self.f._eq, self.w] - data = pickle.dumps(objs) - objs2 = pickle.loads(data) - h2, feq2, w2 = objs2 - self.assertTrue(w2 in h2) - self.assertTrue(feq2 is w2._wref()) - return - - def test_observable_deregistration(self): - """Check if Observable drops dead Observer.""" - f = self.f - x = f.new_parameter("x", 5) - f.set_equation("3 * x") - self.assertEqual(15, f.evaluate()) - self.assertEqual(15, f._eq._value) - # get one of the observer callables that are associated with f - xof = next(iter(x._observers)) - self.assertTrue(isinstance(xof, WeakBoundMethod)) - # changing value of x should reset f._eq - x.set_value(x.value + 1) - self.assertTrue(None is f._eq._value) - self.assertEqual(18, f.evaluate()) - # deallocate f now - self.f = f = None - self.assertTrue(xof in x._observers) - # since f does not exist anymore, the next notification call - # should drop the associated observer. - x.set_value(x.value + 1) - self.assertEqual(0, len(x._observers)) - return - - -# End of class TestWeakBoundMethod - -# Local Routines ------------------------------------------------------------- +# These tests build their objects inside each test instead of in fixtures, +# because pytest keeps fixture values alive and the tests need the object +# behind a weak reference to be garbage collected. def _fallback_example(wbm, *args, **kwargs): return (wbm, args, kwargs) -if __name__ == "__main__": - unittest.main() - -# End of file +def _make_contribution(): + contribution = FitContribution("f") + contribution.set_equation("7") + return contribution + + +# weak_ref stores the fallback it is given, or None without one. +@pytest.mark.parametrize( + "input_fallback, expected_fallback", + [ + # C1: A fallback function is given. + # Expected: The wrapper stores it. + (_fallback_example, _fallback_example), + # C2: No fallback is given. + # Expected: The wrapper's fallback is None. + (None, None), + ], +) +def test_weak_ref_fallback(input_fallback, expected_fallback): + contribution = _make_contribution() + weak_method = weak_ref(contribution._flush, fallback=input_fallback) + actual_fallback = weak_method.fallback + assert actual_fallback is expected_fallback + + +def test_call_live_object(): + # C1: The wrapped method's object is alive and the wrapper is called. + # Expected: The bound method runs; here it flushes the cached value. + contribution = _make_contribution() + weak_flush = weak_ref(contribution._eq._flush) + actual_evaluated_value = contribution.evaluate() + actual_cached_value_before_flush = contribution._eq._value + expected_evaluated_value = 7 + assert actual_evaluated_value == expected_evaluated_value + assert actual_cached_value_before_flush == expected_evaluated_value + weak_flush(()) + actual_cached_value_after_flush = contribution._eq._value + expected_cached_value_after_flush = None + assert actual_cached_value_after_flush == expected_cached_value_after_flush + + +def test_call_returns_method_result(): + # C1: The wrapped method returns a value and its object is alive. + # Expected: The wrapper returns that value. + parameter = Parameter("x", value=3) + weak_get_value = weak_ref(parameter.get_value) + actual_value = weak_get_value() + expected_value = 3 + assert actual_value == expected_value + + +def test_call_dead_object_with_fallback(): + # C1: The wrapped method's object is deleted, then the wrapper is called. + # Expected: The fallback is called with the wrapper and the arguments. + contribution = _make_contribution() + weak_flush = weak_ref(contribution._eq._flush, fallback=_fallback_example) + del contribution + assert weak_flush._wref() is None + actual_call = weak_flush("any", "argument", foo=37) + expected_call = (weak_flush, ("any", "argument"), {"foo": 37}) + assert actual_call[0] is expected_call[0] + assert actual_call[1:] == expected_call[1:] + + +def test_call_dead_object_without_fallback_bad(): + # C1: The wrapped method's object is deleted and there is no fallback. + # Expected: Calling the wrapper raises ReferenceError. + parameter = Parameter("x", value=3) + weak_get_value = weak_ref(parameter.get_value) + del parameter + with pytest.raises(ReferenceError): + weak_get_value() + + +def test_hash(): + # C1: A wrapper's object is deleted, and the wrapper is pickled twice. + # Expected: The hash is unchanged by the deletion, and both pickled + # copies hash equally. + contribution = FitContribution("f1") + weak_flush = weak_ref(contribution._flush) + expected_hash = hash(weak_flush) + del contribution + actual_wrapped_object = weak_flush._wref() + expected_wrapped_object = None + assert actual_wrapped_object == expected_wrapped_object + actual_hash = hash(weak_flush) + assert actual_hash == expected_hash + first_copy = pickle.loads(pickle.dumps(weak_flush)) + second_copy = pickle.loads(pickle.dumps(weak_flush)) + actual_copy_hash = hash(second_copy) + expected_copy_hash = hash(first_copy) + assert actual_copy_hash == expected_copy_hash + + +def test_eq(): + # Two wrappers are equal when they wrap the same method of the same + # object. A pickled copy holds an empty reference, so it only equals the + # original once the original's object has died. + contribution = FitContribution("f1") + first = weak_ref(contribution._flush) + second = weak_ref(contribution._flush) + # C1: Two wrappers of the same live method. + # Expected: They are equal. + actual_is_equal = first == second + expected_is_equal = True + assert actual_is_equal == expected_is_equal + # C2: A pickled copy of a wrapper whose object is alive. + # Expected: The copy has an empty reference and is not equal. + copy = pickle.loads(pickle.dumps(first)) + actual_wrapped_object = copy._wref() + expected_wrapped_object = None + assert actual_wrapped_object == expected_wrapped_object + actual_is_equal = first == copy + expected_is_equal = False + assert actual_is_equal == expected_is_equal + # C3: The original's object is deleted. + # Expected: The original now equals the pickled copy. + del contribution + actual_wrapped_object = first._wref() + expected_wrapped_object = None + assert actual_wrapped_object == expected_wrapped_object + actual_is_equal = first == copy + expected_is_equal = True + assert actual_is_equal == expected_is_equal + # C4: A pickled copy of the pickled copy. + # Expected: It has an empty reference and equals both. + copy_of_copy = pickle.loads(pickle.dumps(copy)) + actual_wrapped_object = copy_of_copy._wref() + expected_wrapped_object = None + assert actual_wrapped_object == expected_wrapped_object + actual_equals_copy = copy_of_copy == copy + actual_equals_first = copy_of_copy == first + expected_is_equal = True + assert actual_equals_copy == expected_is_equal + assert actual_equals_first == expected_is_equal + + +def test_pickling(): + # C1: A set holding a wrapper, the wrapped object and the wrapper are + # pickled together. Unpickling calls the wrapper's __hash__. + # Expected: The unpickled set contains the unpickled wrapper, which + # refers to the unpickled object. + contribution = _make_contribution() + weak_flush = weak_ref(contribution._eq._flush, fallback=_fallback_example) + holder = {weak_flush} + data = pickle.dumps([holder, contribution._eq, weak_flush]) + actual_holder, actual_equation, actual_wrapper = pickle.loads(data) + assert actual_wrapper in actual_holder + assert actual_equation is actual_wrapper._wref() + + +def test_observable_drops_dead_observer(): + # C1: An observer's object is deleted, then the observable notifies. + # Expected: The dead observer is dropped from the observable. + contribution = _make_contribution() + x = contribution.new_parameter("x", 5) + contribution.set_equation("3 * x") + assert contribution.evaluate() == 15 + observer = next(iter(x._observers)) + assert isinstance(observer, WeakBoundMethod) + # Changing x resets the contribution's cached value. + x.set_value(x.value + 1) + assert contribution._eq._value is None + assert contribution.evaluate() == 18 + del contribution + assert observer in x._observers + x.set_value(x.value + 1) + actual_observer_count = len(x._observers) + expected_observer_count = 0 + assert actual_observer_count == expected_observer_count