From 1197ba4fc6d364e961a13443559891365c1258f8 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Fri, 7 Jun 2019 22:28:28 +0100 Subject: [PATCH 1/4] lisa.analysis: Fix save_plot() bbox Avoid cropping the saved plots. --- lisa/analysis/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisa/analysis/base.py b/lisa/analysis/base.py index 71641acd6..25015381a 100644 --- a/lisa/analysis/base.py +++ b/lisa/analysis/base.py @@ -131,7 +131,7 @@ class AnalysisHelpers(Loggable): guessed_format = mime_type.split('/')[1].split('.', 1)[-1].split('+')[0] img_format = img_format or guessed_format - figure.savefig(filepath, format=img_format) + figure.savefig(filepath, format=img_format, bbox_inches='tight') def save_plot(self, figure, filepath=None, img_format=None): """ -- GitLab From c070fa218d9d1855316be207da3457c0b9f8734d Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 11 Jun 2019 12:29:52 +0100 Subject: [PATCH 2/4] lisa.trace: Fix events checker decorator Fix wrapper stack optimisation by updating the inner-most wrapper, instead of bypassing inner wrappers and collecting their events in the outermost one. That avoids undoing other modifications of the wrapper some decorators could have done. --- lisa/trace.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lisa/trace.py b/lisa/trace.py index 5ac9e18d8..6112a190d 100644 --- a/lisa/trace.py +++ b/lisa/trace.py @@ -1148,16 +1148,16 @@ class TraceEventCheckerBase(abc.ABC, Loggable): try: # we want to see through all other kinds of wrappers, down to the # one that matters to us - used_events = inspect.unwrap(f, stop=unwrap_down_to).used_events + unwrapped_f = inspect.unwrap(f, stop=unwrap_down_to) + used_events = unwrapped_f.used_events except AttributeError: checker = self else: + # Update the existing checker inplace to avoid adding an extra + # level of wrappers. checker = AndTraceEventChecker([self, used_events]) - # remove a layer of wrapper if the last layer is a similar wrapper - # to what we are going to install, since we took its used_events - # into account already - if unwrap_down_to(f): - f = f.__wrapped__ + unwrapped_f.used_events = checker + return f sig = inspect.signature(f) if self.check and sig.parameters: @@ -1175,6 +1175,7 @@ class TraceEventCheckerBase(abc.ABC, Loggable): checker.check_events(available_events) return f(self, *args, **kwargs) + # If the decorated object takes no parameters, we cannot check anything else: @wraps(f) -- GitLab From 53fb302117d5566daa54b5c4824d5d583623f4e1 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 11 Jun 2019 12:35:30 +0100 Subject: [PATCH 3/4] utils: Add update_wrapper_doc() helper Behaves like functools.wraps(), with the additional capability of updating the docstring and the signature of the wrapped function. This allows taking into account an "extender" function that is used to implement the wrapper, so it can add some of its parameters to the decorated function, and part of its docstring as well. --- lisa/utils.py | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/lisa/utils.py b/lisa/utils.py index 067768f93..869eeb26c 100644 --- a/lisa/utils.py +++ b/lisa/utils.py @@ -824,4 +824,90 @@ class LayeredMapping(MutableMapping): top=copy.copy(self.top), ) + +def update_wrapper_doc(func, added_by=None, description=None, remove_params=None): + """ + Equivalent to :func:`functools.wraps` that updates the signature by taking + into account the wrapper's extra parameters and the given description. Note + that var positional and var keyword arguments (``*args`` and ``**kwargs``) + are *not* added to the wrapped function's signature, since they are usually + used to transparently forward arguments from the decorator to the decorated + function. + + :param added_by: Add some kind of reference to give a sense of where the + new behaviour of the wraps function comes from. + :type added_by: str or None + + :param description: Extra description output in the docstring. + :type description: str or None + + :param remove_params: Set of parameter names to not include in the decorated + function signature. This can be used to hide parameters that are only + used as part of a decorated/decorator protocol, and not exposed in the + final decorated function. + + .. note:: :func:`functools.wraps` is applied by this decorator, which will + not work if you applied it yourself. + """ + + if description: + description = '\n{}\n'.format(description) + + remove_params = remove_params if remove_params else set() + + def decorator(f): + wrapper_sig = inspect.signature(f) + f = functools.wraps(func)(f) + f_sig = inspect.signature(f) + + added_params = [ + desc + for name, desc in wrapper_sig.parameters.items() + if ( + name not in f_sig.parameters.keys() + # These kinds are usually present in the wrapper to forward + # to the wrapped function, so they are not interesting here + and not desc.kind in ( + inspect.Parameter.VAR_KEYWORD, + inspect.Parameter.VAR_POSITIONAL + ) + ) + ] + + def merge_param_list(l1, l2): + new_list = [] + # Make sure the new param list has the different kinds of + # parameters ordered as it should + for kind in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + inspect.Parameter.VAR_POSITIONAL, + inspect.Parameter.KEYWORD_ONLY, + inspect.Parameter.VAR_KEYWORD, + ): + new_list.extend( + p + # Take from l1 first, then from l2 + for p in itertools.chain(l1, l2) + if p.kind == kind and p.name not in remove_params + ) + return new_list + + f.__signature__ = f_sig.replace( + parameters=merge_param_list(f_sig.parameters.values(), added_params), + ) + + # Replace the one-liner f description + extra_doc = "\n\n{added_by}{description}".format( + added_by='**Added by** {}:\n'.format(added_by) if added_by else '', + description=description if description else '', + ) + + f_doc = inspect.getdoc(f) or '' + f.__doc__ = f_doc + extra_doc + + return f + return decorator + + # vim :set tabstop=4 shiftwidth=4 textwidth=80 expandtab -- GitLab From a61d9dd1c70d3ddcc6bb2f3fec830ab7e98db098 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 11 Jun 2019 12:37:23 +0100 Subject: [PATCH 4/4] lisa.tests.base: Use utils.update_wrapper_doc() Use utils.update_wrapper_doc() in RTATestBundle.check_noisy_tasks(). --- lisa/tests/base.py | 52 +++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/lisa/tests/base.py b/lisa/tests/base.py index 90fd306f8..907e7f75a 100644 --- a/lisa/tests/base.py +++ b/lisa/tests/base.py @@ -35,7 +35,7 @@ from lisa.wlgen.rta import RTA from lisa.utils import ( Serializable, memoized, ArtifactPath, non_recursive_property, - LayeredMapping + LayeredMapping, update_wrapper_doc ) from lisa.trace import FtraceCollector, FtraceConf, DmesgCollector @@ -725,8 +725,20 @@ class RTATestBundle(TestBundle, metaclass=RTATestBundleMeta): function. """ def decorator(func): + @update_wrapper_doc( + func, + added_by=':meth:`lisa.tests.base.RTATestBundle.test_noisy_tasks`', + description=textwrap.dedent( + """ + The returned ``ResultBundle.result`` will be changed to + :attr:`~lisa.tests.base.Result.UNDECIDED` if the environment was + too noisy: + {} + """).strip().format( + inspect.getdoc(cls.test_noisy_tasks) + ) + ) @cls.test_noisy_tasks.used_events - @functools.wraps(func) def wrapper(self, *args, noise_threshold_pct=noise_threshold_pct, noise_threshold_ms=noise_threshold_ms, @@ -742,42 +754,6 @@ class RTATestBundle(TestBundle, metaclass=RTATestBundleMeta): return res - # https://stackoverflow.com/a/33112180 - # The wrapper has all of `func`'s parameters plus `test_noisy_tasks`', - # but since we use `wraps(func)` we'll only get doc/autocompletion for - # `func`'s. Expose the extra parameters to the decorated function to - # make it more user friendly. - func_sig = signature(func) - dec_params = signature(cls.check_noisy_tasks).parameters - - # We want the default values of the new parameters for the - # *decorated* function to be the values passed to the decorator, - # which aren't the default values of the decorator. - new_params = [ - dec_params["noise_threshold_pct"].replace(default=noise_threshold_pct), - dec_params["noise_threshold_ms"].replace(default=noise_threshold_ms) - ] - wrapper.__signature__ = func_sig.replace( - parameters=list(func_sig.parameters.values()) + new_params - ) - - # Make it obvious in the doc where the extra parameters come from - noise_doc = inspect.getdoc(cls.test_noisy_tasks).splitlines() - # Replace the one-liner func description - noise_doc[1] = textwrap.dedent( - """ - **Added by** :meth:`~{}.{}.{}`: - - The returned ``ResultBundle.result`` will be changed to - :attr:`~lisa.tests.base.Result.UNDECIDED` if the environment was - too noisy: - """.format(cls.__module__, cls.__name__, cls.check_noisy_tasks.__name__) - ) - noise_doc = '\n'.join(noise_doc) - - wrapper_doc = inspect.getdoc(wrapper) or '' - wrapper.__doc__ = wrapper_doc + "\n".join(noise_doc) - return wrapper return decorator -- GitLab