-
-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recursive assignment of fields in settings.py to the app.settings object #78
Changes from 2 commits
8938922
483f5ff
480cdc3
6242f9b
b30d1ab
a9c4328
3056976
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,9 @@ | |
from dataclasses import dataclass, field | ||
from datetime import datetime | ||
from typing import Any | ||
import json | ||
from collections import UserDict | ||
|
||
|
||
import pyttman | ||
from pyttman.core.containers import MessageMixin, Reply | ||
|
@@ -32,6 +35,12 @@ def depr_graceful(message: str, version: str): | |
out = f"{message} - This was deprecated in version {version}." | ||
warnings.warn(out, DeprecationWarning) | ||
|
||
class CustomUserDict(UserDict): | ||
|
||
# constructor | ||
def __init__(self, dictionary): | ||
self.data = dictionary | ||
self.__dict__.update(dictionary) | ||
|
||
class Settings: | ||
""" | ||
|
@@ -60,10 +69,20 @@ def __init__(self, **kwargs): | |
self.LOG_FORMAT: str | None = None | ||
self.LOG_TO_STDOUT: bool = False | ||
|
||
[setattr(self, k, v) for k, v in kwargs.items() | ||
[self.__set_attr(k, v) for k, v in kwargs.items() | ||
if not inspect.ismodule(v) | ||
and not inspect.isfunction(v)] | ||
|
||
def __set_attr(self, k, v): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to PEP-8, single |
||
tmp = v | ||
if isinstance(v, dict): | ||
tmp = self.__dict2obj(v) | ||
|
||
setattr(self, k, tmp) | ||
|
||
def __dict2obj(self, dictionary): | ||
return json.loads(json.dumps(dictionary), object_hook=CustomUserDict) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have some thoughts about this method.
|
||
|
||
def __repr__(self): | ||
_attrs = {name: value for name, value in self.__dict__.items()} | ||
return f"Settings({_attrs})" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
foo = "bar" | ||
|
||
d = { | ||
"k1": "v1", | ||
"k2":{"a":"a","b":"b"} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this file is redundant, and the relative import can be troublesome in a shipped package due to its relativity. Can the mockup dict object be placed in the test suite instead, such as in |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
from tests.module_helper import PyttmanInternalBaseTestCase | ||
from pyttman.core.internals import Settings | ||
|
||
from importlib import import_module | ||
from . import mocksettings | ||
|
||
class PyttmanInternalSettingsPyttmanApp(PyttmanInternalBaseTestCase): | ||
def __init__(self, *args, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use |
||
super().__init__(*args, **kwargs) | ||
settings_names = [i for i in dir(mocksettings) | ||
if not i.startswith("_")] | ||
settings_config = {name: getattr(mocksettings, name) | ||
for name in settings_names} | ||
self.settings = Settings(**settings_config) | ||
|
||
def test_read_settings_with_dictionary(self): | ||
|
||
self.assertTrue(self.settings.d.k2.a == "a") | ||
self.assertTrue(self.settings.d["k2"].a == "a") | ||
self.assertTrue(self.settings.d["k2"]["a"] == "a") | ||
|
||
self.assertTrue(self.settings.d.k1 == "v1") | ||
self.assertTrue(self.settings.d["k1"] == "v1") | ||
|
||
self.assertTrue(self.settings.foo == "bar") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this class is redundant, since we already have the
Settings
class which could implement this API instead.. I'd prefer if theSettings
class would handle a recursive assignment of self for all sub-dictionaries, instead of the first level being aSettings
object and all subnodes being of a different type, for no apparent reason.