Repository: datalad/datalad

Issue:
Stop using Appdirs
We rely on [appdirs](https://github.com/ActiveState/appdirs) to figure out appropriate config, data, and cache dirs for any given platform. However, the implementation in appdirs for Windows is for Windows 7 at latest. It relies on [Windows special folders CSIDL IDs](https://www.nirsoft.net/articles/find_special_folder_location.html) and retrieves them for example like this:

```
path = os.path.normpath(_get_win_folder("CSIDL_LOCAL_APPDATA"))
```
where ``CSIDL_LOCAL_APPDATA`` is an identifier for a special folder. This is necessary because Windows versions change the location and name of these special folders.
However, since Windows Vista, these IDs have been replaced by [``KNOWNFOLDERID`` values](https://docs.microsoft.com/en-us/windows/win32/shell/csidl), and Windows advises to use them instead.

I believe this is the underlying cause for the test failure observed in #6160. 

I filed [an issue with appdirs](https://github.com/ActiveState/appdirs/issues/167) this summer because certain CSIDL ID's simply don't exist (they return an empty string on the windows 10/11 test boxes) but did not receive a reply, and it doesn't look like there is much development anymore. 

I think, at least for Windows-related things, we should switch away from appdirs.

Discussion:

Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index be0fff77c..3a537a767 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -134,7 +134,7 @@ since we use it to provide backports of recent fixed external modules we depend
 
 ```sh
 apt-get install -y -q git git-annex-standalone
-apt-get install -y -q patool python3-scrapy python3-{appdirs,argcomplete,git,humanize,keyring,lxml,msgpack,progressbar,requests,setuptools}
+apt-get install -y -q patool python3-scrapy python3-{argcomplete,git,humanize,keyring,lxml,msgpack,progressbar,requests,setuptools}
 ```
 
 and additionally, for development we suggest to use tox and new
diff --git a/datalad/cmdline/helpers.py b/datalad/cmdline/helpers.py
index 1f759f7b0..85012b7fb 100644
--- a/datalad/cmdline/helpers.py
+++ b/datalad/cmdline/helpers.py
@@ -38,7 +38,7 @@ from ..utils import (
     get_suggestions_msg,
 )
 
-from appdirs import AppDirs
+from platformdirs import AppDirs
 from os.path import join as opj
 
 dirs = AppDirs("datalad", "datalad.org")
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/common_cfg.py b/datalad/interface/common_cfg.py
index e45ebeaf8..a4a035599 100644
--- a/datalad/interface/common_cfg.py
+++ b/datalad/interface/common_cfg.py
@@ -12,7 +12,7 @@
 
 __docformat__ = 'restructuredtext'
 
-from appdirs import AppDirs
+from platformdirs import AppDirs
 from os import environ
 from os.path import join as opj, expanduser
 from datalad.support.constraints import EnsureBool
diff --git a/datalad/interface/run_procedure.py b/datalad/interface/run_procedure.py
index 1b35b4c8e..12578a8f5 100644
--- a/datalad/interface/run_procedure.py
+++ b/datalad/interface/run_procedure.py
@@ -216,10 +216,10 @@ class RunProcedure(Interface):
     Directories identified by the configuration settings
 
     - 'datalad.locations.user-procedures' (determined by
-      appdirs.user_config_dir; defaults to '$HOME/.config/datalad/procedures'
+      platformdirs.user_config_dir; defaults to '$HOME/.config/datalad/procedures'
       on GNU/Linux systems)
     - 'datalad.locations.system-procedures' (determined by
-      appdirs.site_config_dir; defaults to '/etc/xdg/datalad/procedures' on
+      platformdirs.site_config_dir; defaults to '/etc/xdg/datalad/procedures' on
       GNU/Linux systems)
     - 'datalad.locations.dataset-procedures'
 
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/cookies.py b/datalad/support/cookies.py
index f93300327..d17c31fd1 100644
--- a/datalad/support/cookies.py
+++ b/datalad/support/cookies.py
@@ -11,7 +11,7 @@
 import atexit
 import logging
 import shelve
-import appdirs
+import platformdirs
 import os.path
 
 from .network import get_tld
@@ -46,7 +46,7 @@ class CookiesDB(object):
             filename = self._filename
             cookies_dir = os.path.dirname(filename)
         else:
-            cookies_dir = os.path.join(appdirs.user_config_dir(), 'datalad')  # FIXME prolly shouldn't hardcode 'datalad'
+            cookies_dir = os.path.join(platformdirs.user_config_dir(), 'datalad')  # FIXME prolly shouldn't hardcode 'datalad'
             filename = os.path.join(cookies_dir, 'cookies')
 
         # TODO: guarantee restricted permissions
diff --git a/datalad/support/external_versions.py b/datalad/support/external_versions.py
index da59b4b18..7a6608e86 100644
--- a/datalad/support/external_versions.py
+++ b/datalad/support/external_versions.py
@@ -161,7 +161,7 @@ class ExternalVersions(object):
     }
     _INTERESTING = (
         'annexremote',
-        'appdirs',
+        'platformdirs',
         'boto',
         'exifread',
         'git',
@@ -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..41b38f2be 100755
--- a/setup.py
+++ b/setup.py
@@ -34,7 +34,7 @@ from _datalad_build_support.setup import (
 
 requires = {
     'core': [
-        'appdirs',
+        'platformdirs',
         'chardet>=3.0.4',      # rarely used but small/omnipresent
         'colorama; platform_system=="Windows"',
         'distro; python_version >= "3.8"',
@@ -44,6 +44,7 @@ requires = {
         'fasteners>=0.14',
         'patool>=1.7',
         'tqdm',
+        'wrapt',
         'annexremote',
     ],
     'downloaders': [


Test patch:
diff --git a/datalad/downloaders/tests/test_providers.py b/datalad/downloaders/tests/test_providers.py
index e20e9f04a..e889e20f4 100644
--- a/datalad/downloaders/tests/test_providers.py
+++ b/datalad/downloaders/tests/test_providers.py
@@ -119,7 +119,7 @@ url_re = https?://crcns\.org/.*
 authentication_type = none
 """}},
    '.git': { "HEAD" : ""}})
-@patch.multiple("appdirs.AppDirs", site_config_dir=None, user_config_dir=None)
+@patch.multiple("platformdirs.AppDirs", site_config_dir=None, user_config_dir=None)
 def test_Providers_from_config__files(sysdir, userdir, dsdir):
     """Test configuration file precedence
 
@@ -143,14 +143,14 @@ def test_Providers_from_config__files(sysdir, userdir, dsdir):
 
         # Test that the system defaults take precedence over the dataset
         # defaults (we're still within the dsdir)
-        with patch.multiple("appdirs.AppDirs", site_config_dir=sysdir, user_config_dir=None):
+        with patch.multiple("platformdirs.AppDirs", site_config_dir=sysdir, user_config_dir=None):
             providers = Providers.from_config_files(reload=True)
             provider = providers.get_provider('https://crcns.org/data....')
             assert_equal(provider.name, 'syscrcns')
 
         # Test that the user defaults take precedence over the system
         # defaults
-        with patch.multiple("appdirs.AppDirs", site_config_dir=sysdir, user_config_dir=userdir):
+        with patch.multiple("platformdirs.AppDirs", site_config_dir=sysdir, user_config_dir=userdir):
             providers = Providers.from_config_files(reload=True)
             provider = providers.get_provider('https://crcns.org/data....')
             assert_equal(provider.name, 'usercrcns')
@@ -158,7 +158,7 @@ def test_Providers_from_config__files(sysdir, userdir, dsdir):
 
 @with_tempfile(mkdir=True)
 def test_providers_enter_new(path):
-    with patch.multiple("appdirs.AppDirs", site_config_dir=None,
+    with patch.multiple("platformdirs.AppDirs", site_config_dir=None,
                         user_config_dir=path):
         providers_dir = op.join(path, "providers")
         providers = Providers.from_config_files(reload=True)
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