core/cachew: special handling for None in order to preserve cache_dir() path

+ add 'suffix' argument for more straighforward logic

my/bluemaestro.py

@@ -52,7 +52,7 @@ def is_bad_table(name: str) -> bool:
 
 from .core.cachew import cache_dir
 from .core.common import mcachew
-@mcachew(depends_on=lambda: inputs(), cache_path=cache_dir() / 'bluemaestro.cache')
+@mcachew(depends_on=lambda: inputs(), cache_path=cache_dir('bluemaestro'))
 def measurements() -> Iterable[Measurement]:
     # todo ideally this would be via arguments... but needs to be lazy
     dbs = inputs()

my/core/cachew.py

@@ -38,6 +38,28 @@ def _appdirs_cache_dir() -> Path:
     return cd
 
 
-def cache_dir() -> Optional[Path]:
+from . import PathIsh
+def cache_dir(suffix: Optional[PathIsh] = None) -> Path:
     from . import core_config as CC
-    return CC.config.get_cache_dir()
+    cdir_ = CC.config.get_cache_dir()
+
+    sp: Optional[Path] = None
+    if suffix is not None:
+        sp = Path(suffix)
+        # guess if you do need absolute, better path it directly instead of as suffix?
+        assert not sp.is_absolute(), sp
+
+    # ok, so ideally we could just return cdir_ / sp
+    # however, this function was at first used without the suffix, e.g. cache_dir() / 'some_dir'
+    # but now cache_dir setting can also be None which means 'disable cache'
+    # changing return type to Optional means that it will break for existing users even if the cache isn't used
+    # it's kinda wrong.. so we use dummy path (_CACHE_DIR_NONE_HACK), and then strip it away in core.common.mcachew
+    # this logic is tested via test_cachew_dir_none
+
+    if cdir_ is None:
+        from .common import _CACHE_DIR_NONE_HACK
+        cdir = _CACHE_DIR_NONE_HACK
+    else:
+        cdir = cdir_
+
+    return cdir if sp is None else cdir / sp

my/core/common.py

@@ -215,6 +215,10 @@ if TYPE_CHECKING:
     mcachew: McachewType
 
 
+_CACHE_DIR_NONE_HACK = Path('/tmp/hpi/cachew_none_hack')
+"""See core.cachew.cache_dir for the explanation"""
+
+
 # TODO I don't really like 'mcachew', just 'cache' would be better... maybe?
 # todo ugh. I think it needs doublewrap, otherwise @mcachew without args doesn't work
 def mcachew(*args, **kwargs): # type: ignore[no-redef]
@@ -222,6 +226,17 @@ def mcachew(*args, **kwargs): # type: ignore[no-redef]
     Stands for 'Maybe cachew'.
     Defensive wrapper around @cachew to make it an optional dependency.
     """
+    cpath = kwargs.get('cache_path')
+    if isinstance(cpath, (str, Path)):
+        try:
+            # check that it starts with 'hack' path
+            Path(cpath).relative_to(_CACHE_DIR_NONE_HACK)
+        except:
+            pass # no action needed, doesn't start with 'hack' string
+        else:
+            # todo show warning? tbh unclear how to detect when user stopped using 'old' way and using suffix instead?
+            # if it does, means that user wanted to disable cache
+            kwargs['cache_path'] = None
     try:
         import cachew
     except ModuleNotFoundError:

tests/misc.py

@@ -120,3 +120,22 @@ def test_cachew() -> None:
 
     assert list(cf()) == [1, 2, 3]
     assert called == cc
+
+
+def test_cachew_dir_none() -> None:
+    from cachew import settings
+    settings.ENABLE = True # by default it's off in tests (see conftest.py)
+
+    from my.core.cachew import cache_dir
+    from my.core.common import mcachew
+    from my.core.core_config import _reset_config as reset
+    with reset() as cc:
+        cc.cache_dir = None
+        called = 0
+        @mcachew(cache_path=cache_dir() / 'ctest')
+        def cf() -> List[int]:
+            nonlocal called
+            called += 1
+            return [called, called, called]
+        assert list(cf()) == [1, 1, 1]
+        assert list(cf()) == [2, 2, 2]