Repository: datalad/datalad

Issue:
Rebalance runtime of windows CI runs on appveyor
![image](https://user-images.githubusercontent.com/136479/141691940-edbfff03-c97f-4261-b534-0bdeb0d6184e.png)

Almost 20min difference between the fastest and the slowest. This is the result of running more tests, and moving tests the other locations in this codebase.

Discussion:

Patch:
diff --git a/.appveyor.yml b/.appveyor.yml
index 91263c547..ca041893f 100644
--- a/.appveyor.yml
+++ b/.appveyor.yml
@@ -105,8 +105,10 @@ environment:
           datalad.cmdline
           datalad.customremotes
           datalad.distribution
+          datalad.distributed
           datalad.downloaders
           datalad.interface
+          datalad.tests
       # TODO: datalad.metadata (this is completely non-functional on Windows,
       #       either tests only, or actual code too)
       APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
@@ -115,9 +117,7 @@ environment:
       # ~50min
       DTS: >
           datalad.local
-          datalad.distributed
           datalad.support
-          datalad.tests
           datalad.ui
       APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
       PY: 39-x64
diff --git a/datalad/distribution/dataset.py b/datalad/distribution/dataset.py
index b92fa0054..6586d632a 100644
--- a/datalad/distribution/dataset.py
+++ b/datalad/distribution/dataset.py
@@ -10,7 +10,6 @@
 """
 
 import logging
-from functools import wraps
 from os.path import (
     curdir,
     exists,
@@ -19,6 +18,7 @@ from os.path import (
     pardir,
 )
 from weakref import WeakValueDictionary
+import wrapt
 
 from datalad import cfg
 from datalad.config import ConfigManager
@@ -447,12 +447,11 @@ def datasetmethod(f, name=None, dataset_argname='dataset'):
 
     The decorator has no effect on the actual function decorated with it.
     """
-
     if not name:
         name = f.__name__
 
-    @wraps(f)
-    def apply_func(instance, *args, **kwargs):
+    @wrapt.decorator
+    def apply_func(wrapped, instance, args, kwargs):
         # Wrapper function to assign arguments of the bound function to
         # original function.
         #
@@ -460,6 +459,7 @@ def datasetmethod(f, name=None, dataset_argname='dataset'):
         # ----
         # This wrapper is NOT returned by the decorator, but only used to bind
         # the function `f` to the Dataset class.
+
         kwargs = kwargs.copy()
         from datalad.utils import getargspec
         orig_pos = getargspec(f).args
@@ -484,12 +484,7 @@ def datasetmethod(f, name=None, dataset_argname='dataset'):
                 kwargs[orig_pos[i+1]] = args[i]
         return f(**kwargs)
 
-    setattr(Dataset, name, apply_func)
-    # set the ad-hoc attribute so that @build_doc could also bind built doc
-    # to the dataset method
-    if getattr(f, '_dataset_method', None):
-        raise RuntimeError(f"_dataset_method of {f} is already set to {f._dataset_method}")
-    setattr(f, '_dataset_method', apply_func)
+    setattr(Dataset, name, apply_func(f))
     return f
 
 
diff --git a/datalad/interface/base.py b/datalad/interface/base.py
index 19a8b7b42..f9a4df024 100644
--- a/datalad/interface/base.py
+++ b/datalad/interface/base.py
@@ -525,9 +525,6 @@ def build_doc(cls, **kwargs):
         add_args=add_args
     )
 
-    if hasattr(cls.__call__, '_dataset_method'):
-        cls.__call__._dataset_method.__doc__ = cls.__call__.__doc__
-
     # return original
     return cls
 
diff --git a/datalad/interface/utils.py b/datalad/interface/utils.py
index a76375941..8ff2bcef5 100644
--- a/datalad/interface/utils.py
+++ b/datalad/interface/utils.py
@@ -15,9 +15,9 @@ __docformat__ = 'restructuredtext'
 import inspect
 import logging
 import os
+import wrapt
 import sys
 import re
-from functools import wraps
 from time import time
 from os import curdir
 from os import pardir
@@ -281,7 +281,7 @@ def get_result_filter(fx):
     return _fx
 
 
-def eval_results(wrapped):
+def eval_results(func):
     """Decorator for return value evaluation of datalad commands.
 
     Note, this decorator is only compatible with commands that return
@@ -322,9 +322,9 @@ def eval_results(wrapped):
       i.e. a datalad command definition
     """
 
-    @wraps(wrapped)
-    def eval_func(*args, **kwargs):
-        lgr.log(2, "Entered eval_func for %s", wrapped)
+    @wrapt.decorator
+    def eval_func(wrapped, instance, args, kwargs):
+        lgr.log(2, "Entered eval_func for %s", func)
         # for result filters
         # we need to produce a dict with argname/argvalue pairs for all args
         # incl. defaults and args given as positionals
@@ -469,9 +469,9 @@ def eval_results(wrapped):
             lgr.log(2, "Returning generator_func from eval_func for %s", wrapped_class)
             return generator_func(*args, **kwargs)
         else:
-            @wraps(generator_func)
-            def return_func(*args_, **kwargs_):
-                results = generator_func(*args_, **kwargs_)
+            @wrapt.decorator
+            def return_func(wrapped_, instance_, args_, kwargs_):
+                results = wrapped_(*args_, **kwargs_)
                 if inspect.isgenerator(results):
                     # unwind generator if there is one, this actually runs
                     # any processing
@@ -483,9 +483,9 @@ def eval_results(wrapped):
                     return results
 
             lgr.log(2, "Returning return_func from eval_func for %s", wrapped_class)
-            return return_func(*args, **kwargs)
+            return return_func(generator_func)(*args, **kwargs)
 
-    ret = eval_func
+    ret = eval_func(func)
     ret._eval_results = True
     return ret
 
diff --git a/datalad/support/external_versions.py b/datalad/support/external_versions.py
index da59b4b18..055af4e23 100644
--- a/datalad/support/external_versions.py
+++ b/datalad/support/external_versions.py
@@ -176,6 +176,7 @@ class ExternalVersions(object):
         'cmd:7z',
         'requests',
         'scrapy',
+        'wrapt',
     )
 
     def __init__(self):
diff --git a/datalad/utils.py b/datalad/utils.py
index 59ef77502..5037bd50d 100644
--- a/datalad/utils.py
+++ b/datalad/utils.py
@@ -26,6 +26,7 @@ import gzip
 import stat
 import string
 import warnings
+import wrapt
 
 import os.path as op
 
@@ -157,59 +158,10 @@ ArgSpecFake = collections.namedtuple(
     "ArgSpecFake", ["args", "varargs", "keywords", "defaults"])
 
 
-def getargspec(func, *, include_kwonlyargs=False):
-    """Compat shim for getargspec deprecated in python 3.
-
-    The main difference from inspect.getargspec (and inspect.getfullargspec
-    for that matter) is that by using inspect.signature we are providing
-    correct args/defaults for functools.wraps'ed functions.
-
-    `include_kwonlyargs` option was added to centralize getting all args,
-    even the ones which are kwonly (follow the ``*,``).
-
-    For internal use and not advised for use in 3rd party code.
-    Please use inspect.signature directly.
-    """
-    # We use signature, and not getfullargspec, because only signature properly
-    # "passes" args from a functools.wraps decorated function.
-    # Note: getfullargspec works Ok on wrapt-decorated functions
-    f_sign = inspect.signature(func)
-    # Loop through parameters and compose argspec
-    args4 = [[], None, None, {}]
-    # Collect all kwonlyargs into a dedicated dict - name: default
-    kwonlyargs = {}
-    # shortcuts
-    args, defaults = args4[0], args4[3]
-    P = inspect.Parameter
-
-    for p_name, p in f_sign.parameters.items():
-        if p.kind in (P.POSITIONAL_ONLY, P.POSITIONAL_OR_KEYWORD):
-            assert not kwonlyargs  # yoh: must not come after kwonlyarg
-            args.append(p_name)
-            if p.default is not P.empty:
-                defaults[p_name] = p.default
-        elif p.kind == P.VAR_POSITIONAL:
-            args4[1] = p_name
-        elif p.kind == P.VAR_KEYWORD:
-            args4[2] = p_name
-        elif p.kind == P.KEYWORD_ONLY:
-            assert p.default is not P.empty
-            kwonlyargs[p_name] = p.default
-
-    if kwonlyargs:
-        if not include_kwonlyargs:
-            raise ValueError(
-                'Function has keyword-only parameters or annotations, either use '
-                'inspect.signature() API which can support them, or provide include_kwonlyargs=True '
-                'to this function'
-            )
-        else:
-            args.extend(list(kwonlyargs))
-            defaults.update(kwonlyargs)
-
-    # harmonize defaults to how original getargspec returned them -- just a tuple
-    args4[3] = None if not defaults else tuple(defaults.values())
-    return ArgSpecFake(*args4)
+def getargspec(func):
+    """Minimal compat shim for getargspec deprecated in python 3.
+    """
+    return ArgSpecFake(*inspect.getfullargspec(func)[:4])
 
 
 def any_re_search(regexes, value):
@@ -1061,12 +1013,20 @@ def saved_generator(gen):
 #
 # Decorators
 #
+def better_wraps(to_be_wrapped):
+    """Decorator to replace `functools.wraps`
 
-# Originally better_wraps was created to provide `wrapt`-based, instead of
-# `functools.wraps` implementation to preserve the correct signature of the
-# decorated function. By using inspect.signature in our getargspec, which
-# works fine on `functools.wraps`ed functions, we mediated this necessity.
-better_wraps = wraps
+    This is based on `wrapt` instead of `functools` and in opposition to `wraps`
+    preserves the correct signature of the decorated function.
+    It is written with the intention to replace the use of `wraps` without any
+    need to rewrite the actual decorators.
+    """
+
+    @wrapt.decorator(adapter=to_be_wrapped)
+    def intermediator(to_be_wrapper, instance, args, kwargs):
+        return to_be_wrapper(*args, **kwargs)
+
+    return intermediator
 
 
 # Borrowed from pandas
@@ -1149,14 +1109,15 @@ def collect_method_callstats(func):
       - .repo is expensive since does all kinds of checks.
       - .config is expensive transitively since it calls .repo each time
 
+
     TODO:
-      - fancy one could look through the stack for the same id(self) to see if
-        that location is already in memo.  That would hint to the cases where object
-        is not passed into underlying functions, causing them to redo the same work
-        over and over again
-      - ATM might flood with all "1 lines" calls which are not that informative.
-        The underlying possibly suboptimal use might be coming from their callers.
-        It might or not relate to the previous TODO
+    - fancy one could look through the stack for the same id(self) to see if
+      that location is already in memo.  That would hint to the cases where object
+      is not passed into underlying functions, causing them to redo the same work
+      over and over again
+    - ATM might flood with all "1 lines" calls which are not that informative.
+      The underlying possibly suboptimal use might be coming from their callers.
+      It might or not relate to the previous TODO
     """
     from collections import defaultdict
     import traceback
diff --git a/requirements-devel.txt b/requirements-devel.txt
index 1fb3115d7..d64ccc9f3 100644
--- a/requirements-devel.txt
+++ b/requirements-devel.txt
@@ -1,4 +1,7 @@
 # Theoretically we don't want -e here but ATM pip would puke if just .[full] is provided
 # Since we use requirements.txt ATM only for development IMHO it is ok but
 # we need to figure out/complaint to pip folks
+# For now, until https://github.com/GrahamDumpleton/wrapt/issues/98 resolved
+# we should use fresh version allowing to disable extensions
+wrapt>=1.10.11
 -e .[devel]
diff --git a/setup.py b/setup.py
index 704316877..cc547130d 100755
--- a/setup.py
+++ b/setup.py
@@ -44,6 +44,7 @@ requires = {
         'fasteners>=0.14',
         'patool>=1.7',
         'tqdm',
+        'wrapt',
         'annexremote',
     ],
     'downloaders': [


Test patch:
diff --git a/datalad/tests/test_utils.py b/datalad/tests/test_utils.py
index 58dd659ce..cdf86de75 100644
--- a/datalad/tests/test_utils.py
+++ b/datalad/tests/test_utils.py
@@ -11,8 +11,6 @@
 
 """
 
-from functools import wraps
-import inspect
 import os
 import os.path as op
 import shutil
@@ -51,7 +49,6 @@ from datalad.utils import (
     file_basename,
     find_files,
     generate_chunks,
-    getargspec,
     get_dataset_root,
     get_open_files,
     get_path_prefix,
@@ -125,6 +122,9 @@ from datalad import cfg as dl_cfg
 
 
 def test_better_wraps():
+    from functools import wraps
+    from datalad.utils import getargspec
+
     def wraps_decorator(func):
         @wraps(func)
         def  _wrap_wraps_decorator(*args, **kwargs):
@@ -148,69 +148,11 @@ def test_better_wraps():
         return "function2"
 
     eq_("function1", function1(1, 2, 3))
-    # getargspec shim now can handle @wraps'ed functions just fine
-    eq_(getargspec(function1)[0], ['a', 'b', 'c'])
+    eq_(getargspec(function1)[0], [])
     eq_("function2", function2(1, 2, 3))
     eq_(getargspec(function2)[0], ['a', 'b', 'c'])
 
 
-def test_getargspec():
-
-    def eq_argspec(f, expected, has_kwonlyargs=False):
-        """A helper to centralize testing of getargspec on original and wrapped function
-
-        has_kwonlyargs is to instruct if function has kwonly args so we do not try to compare
-        to inspect.get*spec functions, which would barf ValueError if attempted to run on a
-        function with kwonlys. And also we pass it as include_kwonlyargs to our getargspec
-        """
-        # so we know that our expected is correct
-        if not has_kwonlyargs:
-            # if False - we test function with kwonlys - inspect.getargspec would barf
-            eq_(inspect.getargspec(f), expected)
-            # and getfullargspec[:4] wouldn't provide a full picture
-            eq_(inspect.getfullargspec(f)[:4], expected)
-        else:
-            assert_raises(ValueError, inspect.getargspec, f)
-            inspect.getfullargspec(f)  # doesn't barf
-        eq_(getargspec(f, include_kwonlyargs=has_kwonlyargs), expected)
-
-        # and lets try on a wrapped one -- only ours can do the right thing
-        def decorator(f):
-            @wraps(f)
-            def wrapper(*args, **kwargs):
-                return f(*args, **kwargs)
-            return wrapper
-        fw = decorator(f)
-        if has_kwonlyargs:
-            # We barf ValueError similarly to inspect.getargspec, unless explicitly requested
-            # to include kwonlyargs
-            assert_raises(ValueError, getargspec, fw)
-        eq_(getargspec(fw, include_kwonlyargs=has_kwonlyargs), expected)
-
-    def f0():
-        pass
-
-    yield eq_argspec, f0, ([], None, None, None)
-
-    def f1(a1, kw1=None, kw0=1):
-        pass
-
-    yield eq_argspec, f1, (['a1', 'kw1', 'kw0'], None, None, (None, 1))
-
-    # Having *a already makes keyword args to be kwonlyargs, in that
-    # inspect.get*spec would barf
-    def f1_args(a1, *a, kw1=None, kw0=1, **kw):
-        pass
-
-    yield eq_argspec, f1_args, (['a1', 'kw1', 'kw0'], 'a', 'kw', (None, 1)), True
-
-    def f1_star(a1, *, kw1=None, kw0=1):
-        pass
-
-    assert_raises(ValueError, getargspec, f1_star)
-    yield eq_argspec, f1_star, (['a1', 'kw1', 'kw0'], None, None, (None, 1)), True
-
-
 @with_tempfile(mkdir=True)
 def test_rotree(d):
     d2 = opj(d, 'd1', 'd2')  # deep nested directory