From 522bfff679af4e5a0136decf84e0fa0620d1e453 Mon Sep 17 00:00:00 2001
From: Dima Gerasimov <karlicoss@gmail.com>
Date: Wed, 13 May 2020 20:23:29 +0100
Subject: [PATCH] update configuration doc & more tests

---
 doc/CONFIGURING.org | 13 ++++++++-----
 my/demo.py          | 15 +++++----------
 tests/demo.py       | 35 ++++++++++++++++++++++++++++++++++-
 3 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/doc/CONFIGURING.org b/doc/CONFIGURING.org
index 6c7c70e..83342e0 100644
--- a/doc/CONFIGURING.org
+++ b/doc/CONFIGURING.org
@@ -224,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
@@ -256,24 +256,27 @@ I claim this solves pretty much everything:
 - *(6)*: the dataclass header is easily readable, and it's possible to generate the docs automatically
 
 Downsides:
-- inheriting from ~user_config~ means early import of =my.config=
+- inheriting from ~user_config~ means an 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?
+  and it's [[https://github.com/karlicoss/HPI/blob/1e6e0bd381d20437343473878c7f63b1f9d6362b/tests/demo.py#L22-L25][possible]] to do something dynamic like =del sys.modules['my.bluemastro']= to reload the config, I think it's a minor issue.
 
 - =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.
+  Perhaps it will be better when [[https://github.com/python/mypy/issues/5374][this mypy issue]] 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]]
 
+  # In addition, it's not even necessary if you don't have optional attributes, you can simply use the class variables (i.e. ~bluemaestro.export_path~)
+  # upd. ugh, you can't, it doesn't handle default attributes overriding correctly (see tests/demo.py)
+  # eh. basically all I need is class level dataclass??
+
 - 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~.
diff --git a/my/demo.py b/my/demo.py
index ae57b67..811e9e2 100644
--- a/my/demo.py
+++ b/my/demo.py
@@ -16,13 +16,8 @@ class demo(user_config):
     username: str
     timezone: tzinfo = pytz.utc
 
-
-def config() -> demo:
-    from .core.cfg import make_config
-    config = make_config(demo)
-    return config
-
-
+from .core.cfg import make_config
+config = make_config(demo)
 
 from pathlib import Path
 from typing import Sequence, Iterable
@@ -40,17 +35,17 @@ class Item:
 
 
 def inputs() -> Sequence[Path]:
-    return get_files(config().data_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)
+        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,
+                username=config.username,
                 raw=raw,
                 dt=dt,
             )
diff --git a/tests/demo.py b/tests/demo.py
index 93b777c..4dfae6d 100644
--- a/tests/demo.py
+++ b/tests/demo.py
@@ -54,7 +54,40 @@ def test_dynamic_config_simplenamespace(tmp_path: Path) -> None:
     my.config.demo = user_config # type: ignore[misc, assignment]
 
     from my.demo import config
-    assert config().username == 'user3'
+    assert config.username == 'user3'
+
+
+# make sure our config handling pattern does it as expected
+def test_attribute_handling(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 pytz
+    nytz = pytz.timezone('America/New_York')
+
+    import my.config
+    class user_config:
+        # check that override is taken into the account
+        timezone = nytz
+
+        irrelevant = 'hello'
+
+        username = 'UUU'
+        data_path = f'{tmp_path}/*.json'
+
+
+    my.config.demo = user_config # type: ignore[misc, assignment]
+
+    from my.demo import config
+
+    assert config.username == 'UUU'
+
+    # mypy doesn't know about it, but the attribute is there
+    assert getattr(config, 'irrelevant') == 'hello'
+
+    # check that overriden default attribute is actually getting overridden
+    assert config.timezone == nytz