Merge pull request #47 from karlicoss/more-documentation

More documentation & tests

doc/CONFIGURING.org

@@ -20,11 +20,11 @@ Now, the requirements as I see it:
 
 1. configuration should be *extremely* flexible
 
-   We need to make sure it's very easy to combine/filter/extend data without having to modify and rewrite the module code.
+   We need to make sure it's very easy to combine/filter/extend data without having to turn the module code inside out.
    This means using a powerful language for config, and realistically, a Turing complete.
 
    General: that means that you should be able to use powerful syntax, potentially running arbitrary code if
-   this is something you need (for whatever mad reason). It should be possible to override config attributes in runtime, if necessary.
+   this is something you need (for whatever mad reason). It should be possible to override config attributes *in runtime*, if necessary, without rewriting files on the filesystem.
 
    Specific: we've got Python already, so it makes a lot of sense to use it!
 
@@ -160,6 +160,16 @@ from my.config import bluemaestro as user_config
 Let's go through requirements:
 
 - (1): *yes*, simply importing Python code is the most flexible you can get
+  In addition, in runtime, you can simply assign a new config if you need some dynamic hacking:
+
+  #+begin_src python
+  class new_config:
+      export_path = '/some/hacky/dynamic/path'
+  my.config = new_config
+  #+end_src
+
+  After that, =my.bluemaestro= would run against your new config.
+
 - (2): *no*, but backwards compatibility is not necessary in the first version of the module
 - (3): *mostly*, although optional fields require extra work
 - (4): *yes*, whatever is in the config can immediately be used by the code
@@ -176,7 +186,7 @@ I see mypy annotations as the only sane way to support it, because we also get (
   However, it's using plain files and doesn't satisfy (1).
 
   Also not sure about (5). =file-config= allows using mypy annotations, but I'm not convinced they would be correctly typed with mypy, I think you need a plugin for that.
- 
+
 - [[https://mypy.readthedocs.io/en/stable/protocols.html#simple-user-defined-protocols][Protocol]]
 
   I experimented with ~Protocol~ [[https://github.com/karlicoss/HPI/pull/45/commits/90b9d1d9c15abe3944913add5eaa5785cc3bffbc][here]].
@@ -187,6 +197,8 @@ I see mypy annotations as the only sane way to support it, because we also get (
   - it doesn't support optional attributes (optional as in non-required, not as ~typing.Optional~), so it goes against (3)
   - prior to python 3.8, it's a part of =typing_extensions= rather than standard =typing=, so using it requires guarding the code with =if typing.TYPE_CHECKING=, which is a bit confusing and bloating.
 
+  TODO: check out [[https://mypy.readthedocs.io/en/stable/protocols.html#using-isinstance-with-protocols][@runtime_checkable]]?
+
 - =NamedTuple=
 
   [[https://github.com/karlicoss/HPI/pull/45/commits/c877104b90c9d168eaec96e0e770e59048ce4465][Here]] I experimented with using ~NamedTuple~.
@@ -203,7 +215,7 @@ I see mypy annotations as the only sane way to support it, because we also get (
 
   Downsides:
   - we partially lost (5), because dynamic attributes are not transparent to mypy.
-   
+
 
 My conclusion was using a *combined approach*:
 
@@ -212,7 +224,7 @@ My conclusion was using a *combined approach*:
 
 Inheritance is a standard mechanism, which doesn't require any extra frameworks and plays well with other Python concepts. As a specific example:
 
-#+begin_src python
+,#+begin_src python
 from my.config import bluemaestro as user_config
 
 @dataclass
@@ -232,7 +244,7 @@ class bluemaestro(user_config):
         }
         return cls(**params)
 
-config = reddit.make_config()
+config = bluemaestro.make_config()
 #+end_src
 
 I claim this solves pretty much everything:
@@ -240,11 +252,32 @@ I claim this solves pretty much everything:
 - *(2)*: collaterally, we also solved it, because we can adapt for renames and other legacy config adaptations in ~make_config~
 - *(3)*: supports default attributes, at no extra cost
 - *(4)*: the user config's attributes are available through the base class
-- *(5)*: everything is transparent to mypy. However, it still lacks runtime checks.
+- *(5)*: everything is mostly transparent to mypy. There are no runtime type checks yet, but I think possible to integrate with ~@dataclass~
 - *(6)*: the dataclass header is easily readable, and it's possible to generate the docs automatically
 
 Downsides:
-- the =make_config= bit is a little scary and manual, however, it can be extracted in a generic helper method
+- inheriting from ~user_config~ means early import of =my.config=
+
+  Generally it's better to keep everything as lazy as possible and defer loading to the first time the config is used.
+  This might be annoying at times, e.g. if you have a top-level import of you module, but no config.
+
+  But considering that in 99% of cases config is going to be on the disk
+  and it's possible to do something dynamic like =del sys.modules['my.bluemastro']= to reload the config, I think it's a minor issue.
+  # TODO demonstrate in a test?
+
+- =make_config= allows for some mypy false negatives in the user config
+
+  E.g. if you forgot =export_path= attribute, mypy would miss it. But you'd have a runtime failure, and the downstream code using config is still correctly type checked.
+
+  Perhaps it will be better when [[https://github.com/python/mypy/issues/5374][this]] is fixed.
+- the =make_config= bit is a little scary and manual
+
+  However, it's extracted in a generic helper, and [[https://github.com/karlicoss/HPI/blob/d6f071e3b12ba1cd5a86ad80e3821bec004e6a6d/my/twitter/archive.py#L17][ends up pretty simple]]
+
+- inheriting from ~user_config~ requires it to be a =class= rather than an =object=
+
+  A practical downside is you can't use something like ~SimpleNamespace~.
+  But considering you can define an ad-hoc =class= anywhere, this is fine?
 
 My conclusion is that I'm going with this approach for now.
 Note that at no stage in required any changes to the user configs, so if I missed something, it would be reversible.

my/demo.py

@@ -0,0 +1,56 @@
+'''
+Just a demo module for testing and documentation purposes
+'''
+
+from .core.common import Paths
+
+from datetime import tzinfo
+import pytz
+
+from my.config import demo as user_config
+from dataclasses import dataclass
+
+@dataclass
+class demo(user_config):
+    data_path: Paths
+    username: str
+    timezone: tzinfo = pytz.utc
+
+
+def config() -> demo:
+    from .core.cfg import make_config
+    config = make_config(demo)
+    return config
+
+
+
+from pathlib import Path
+from typing import Sequence, Iterable
+from datetime import datetime
+from .core.common import Json, get_files
+
+@dataclass
+class Item:
+    '''
+    Some completely arbirary artificial stuff, just for testing
+    '''
+    username: str
+    raw: Json
+    dt: datetime
+
+
+def inputs() -> Sequence[Path]:
+    return get_files(config().data_path)
+
+
+import json
+def items() -> Iterable[Item]:
+    for f in inputs():
+        dt = datetime.fromtimestamp(f.stat().st_mtime, tz=config().timezone)
+        j = json.loads(f.read_text())
+        for raw in j:
+            yield Item(
+                username=config().username,
+                raw=raw,
+                dt=dt,
+            )

setup.py

@@ -60,7 +60,11 @@ def main():
                 # TODO document these?
                 'logzero',
                 'cachew',
-            ]
+            ],
+            ':python_version<"3.7"': [
+                # used for some modules... hopefully reasonable to have it as a default
+                'dataclasses',
+            ],
         },
     )
 

tests/config.py

@@ -1,6 +1,8 @@
 from pathlib import Path
 
 
+# TODO switch these from using SimpleNamespace
+
 def setup_notes_path(notes: Path) -> None:
     # TODO reuse doc from my.cfg?
     from my.cfg import config

tests/demo.py

@@ -0,0 +1,69 @@
+import sys
+from pathlib import Path
+from more_itertools import ilen
+
+# TODO NOTE: this wouldn't work because of an early my.config.demo import
+# from my.demo import items
+
+def test_dynamic_config_1(tmp_path: Path) -> None:
+    import my.config
+
+    class user_config:
+        username  = 'user'
+        data_path = f'{tmp_path}/*.json'
+    my.config.demo = user_config # type: ignore[misc, assignment]
+
+    from my.demo import items
+    [item1, item2] = items()
+    assert item1.username == 'user'
+
+
+# exactly the same test, but using a different config, to test out the behavious w.r.t. import order
+def test_dynamic_config_2(tmp_path: Path) -> None:
+    # doesn't work without it!
+    # because the config from test_dybamic_config_1 is cached in my.demo.demo
+    del sys.modules['my.demo']
+
+    import my.config
+
+    class user_config:
+        username  = 'user2'
+        data_path = f'{tmp_path}/*.json'
+    my.config.demo = user_config # type: ignore[misc, assignment]
+
+    from my.demo import items
+    [item1, item2] = items()
+    assert item1.username == 'user2'
+
+
+import pytest # type: ignore
+
+@pytest.mark.skip(reason="won't work at the moment because of inheritance")
+def test_dynamic_config_simplenamespace(tmp_path: Path) -> None:
+    # doesn't work without it!
+    # because the config from test_dybamic_config_1 is cached in my.demo.demo
+    del sys.modules['my.demo']
+
+    import my.config
+    from types import SimpleNamespace
+
+    user_config = SimpleNamespace(
+        username='user3',
+        data_path=f'{tmp_path}/*.json',
+    )
+    my.config.demo = user_config # type: ignore[misc, assignment]
+
+    from my.demo import config
+    assert config().username == 'user3'
+
+
+
+@pytest.fixture(autouse=True)
+def prepare(tmp_path: Path):
+    (tmp_path / 'data.json').write_text('''
+[
+    {"key1": 1},
+    {"key2": 2}
+]
+''')
+    yield

tox.ini

@@ -13,7 +13,7 @@ commands =
     # todo these are probably not necessary anymore?
     python3 -c 'from my.config import stub as config; print(config.key)'
     python3 -c 'import my.config; import my.config.repos' # shouldn't fail at least
-    python3 -m pytest tests/misc.py tests/get_files.py tests/config.py::test_set_repo tests/config.py::test_environment_variable
+    python3 -m pytest tests/misc.py tests/get_files.py tests/config.py::test_set_repo tests/config.py::test_environment_variable tests/demo.py
     # TODO add; once I figure out porg depdencency?? tests/config.py
     # TODO run demo.py? just make sure with_my is a bit cleverer?
     # TODO e.g. under CI, rely on installing