-
Notifications
You must be signed in to change notification settings - Fork 79
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
Implement #128: runtime-editable config for default parameters and module settings #185
base: master
Are you sure you want to change the base?
Changes from all commits
f473b80
b0db4bf
bd675f6
6602a0f
f97df73
1cbcad8
dbb1caa
3e3d2e2
73110ed
b0a488f
9a88d12
69ee1c0
8cc7289
6d657ef
fb93906
5d3ef16
1874671
603ce7e
f21246e
d563fa5
4ba5a65
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 |
---|---|---|
|
@@ -107,3 +107,4 @@ venv.bak/ | |
.vscode/ | ||
vscode/ | ||
*.code-workspace | ||
.vscode-* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
[MESSAGES CONTROL] | ||
|
||
disable=C0103,C0114,C0115,C0116,W0613 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,15 @@ | ||
from .internal import get_default_lang, set_default_lang, get_default_loc, \ | ||
get_active_langs, _set_active_langs, get_primary_lang_code, \ | ||
get_full_lang_code, resolve_resource_file, load_language, \ | ||
load_languages, unload_language, unload_languages, get_supported_langs | ||
### DO NOT CHANGE THIS IMPORT ORDER ### | ||
from .internal import get_active_langs, get_supported_locs, \ | ||
get_full_lang_code, get_supported_langs, get_default_loc, \ | ||
get_primary_lang_code | ||
Comment on lines
+2
to
+4
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. Can we move to using full words in variable names? I think this also reduces the cognitive load when reading through code - particularly for people unfamiliar with it. 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'm all for standardizing.
I was thinking 'locale', but I didn't give it too much thought. I am by no means married to the names. |
||
|
||
from lingua_franca import config | ||
from .configuration import Config | ||
|
||
### END OF IMPORT ORDER ### | ||
|
||
from .internal import get_default_lang, set_default_lang, \ | ||
_set_active_langs, resolve_resource_file, \ | ||
load_language, load_languages, unload_language, unload_languages | ||
|
||
|
||
config = Config() |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
import json | ||
|
||
from lingua_franca import get_active_langs, get_supported_locs, \ | ||
get_supported_langs, get_primary_lang_code, get_full_lang_code, \ | ||
get_default_loc | ||
from lingua_franca.internal import resolve_resource_file | ||
|
||
default_global_values = \ | ||
{ | ||
'load_langs_on_demand': False | ||
} | ||
|
||
|
||
class LangConfig(dict): | ||
def __init__(self, lang_code): | ||
super().__init__() | ||
if lang_code not in get_supported_locs(): | ||
# DO NOT catch UnsupportedLanguageError! | ||
# If this fails, we want to crash. This can *only* result from | ||
# someone trying to override sanity checks upstairs. There are no | ||
# circumstances under which this should fail and allow the program | ||
# to continue. | ||
lang_code = get_full_lang_code(lang_code) | ||
|
||
resource_file = resolve_resource_file(f'text/{lang_code}/config.json') | ||
try: | ||
with open(resource_file, 'r', encoding='utf-8') as i_file: | ||
default_values = json.load(i_file) | ||
for k in default_values: | ||
self[k] = default_values[k] | ||
except (FileNotFoundError, TypeError): | ||
pass | ||
|
||
|
||
class Config(dict): | ||
def __init__(self): | ||
super().__init__() | ||
self['global'] = dict(default_global_values) | ||
for lang in get_active_langs(): | ||
self.load_lang(lang) | ||
|
||
def load_lang(self, lang): | ||
if all((lang not in get_supported_locs(), | ||
lang in get_supported_langs())): | ||
if lang not in self.keys(): | ||
self[lang] = {} | ||
self[lang]['universal'] = LangConfig(lang) | ||
|
||
full_loc = get_full_lang_code(lang) | ||
else: | ||
full_loc = lang | ||
lang = get_primary_lang_code(lang) | ||
|
||
self[lang][full_loc] = LangConfig(full_loc) | ||
|
||
def _find_setting(self, setting=None, lang=''): | ||
if setting is None: | ||
raise ValueError("lingua_franca.config requires " | ||
"a setting parameter!") | ||
|
||
setting_available_in = [] | ||
possible_locs = [] | ||
|
||
while True: | ||
if setting in self['global']: | ||
setting_available_in.append('global') | ||
if lang == 'global': | ||
break | ||
|
||
lang = lang or get_default_loc() | ||
|
||
if lang in get_supported_langs(): | ||
possible_locs.append((lang, 'universal')) | ||
possible_locs.append((lang, get_full_lang_code(lang))) | ||
|
||
if lang in get_supported_locs(): | ||
primary_lang_code = get_primary_lang_code(lang) | ||
possible_locs.append((primary_lang_code, 'universal')) | ||
possible_locs.append( | ||
(primary_lang_code, get_default_loc(primary_lang_code))) | ||
possible_locs.append((primary_lang_code, lang)) | ||
|
||
for place in possible_locs: | ||
if setting in self[place[0]][place[1]]: | ||
setting_available_in.append(place) | ||
|
||
break | ||
try: | ||
return setting_available_in[-1] | ||
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. why do we take the last one? I haven't reasoned this through completely but I assume the order would roughly be:
Is that right? If so it seems like we would want the default lang setting over an other locale setting. But I could be getting the whole thing mixed up. Given that I need to reason about it we should document what is intended in the docstring and write some tests for it. 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. This is definitely one of the things that will need the most discussion, but you're about right re: priority. The list is just in the opposite order of what you're thinking =P 'place' might not be a great variable name in this context, but it's referring to the possible indices of the desired setting, nothing to do with LF's job. Could be in this dict or this dict or that dict...
If the setting is found at all,
If the value isn't present in any of those places, it's unavailable in that language. 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. Perusing stales, rephrased. At runtime, LF config is a dict of dicts. It's got settings for the whole module and settings for each locale. Each language defers to a particular locale as its "default" locale. This is configurable. Gez's default English locale will be en-AU, Chance's will be en-US, Aditya might change it up on the fly! So. We want to
This way, a "more local" config dict will always override its less-local counterpart, assuming the requested key is present in the more local dict. 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. For the sake of populating each field, let's say I am running with
If I ask for an es-US setting that's fully localized, I expect
representing, per the previous comment,
but, if any of the config dicts are missing the setting I asked for, they'll be absent from this list and LF will move on with its life, returning instead the next value "up" the chain. At the end, this bit would return |
||
except IndexError: | ||
return None | ||
|
||
def get(self, setting=None, lang=''): | ||
if lang != 'global': | ||
if all((lang, | ||
get_primary_lang_code(lang) not in get_active_langs())): | ||
raise ModuleNotFoundError(f"{lang} is not currently loaded") | ||
|
||
try: | ||
setting_location = self._find_setting(setting, lang) | ||
|
||
if setting_location == 'global': | ||
return self['global'][setting] | ||
return self[setting_location[0]][setting_location[1]][setting] | ||
|
||
except TypeError: | ||
return None | ||
|
||
def set(self, setting=None, value=None, lang='global'): | ||
if lang == 'global': | ||
if setting in self['global']: | ||
self['global'][setting] = value | ||
return | ||
|
||
setting_location = self._find_setting(setting, lang if lang != | ||
'global' else get_default_loc()) | ||
if all((setting_location, setting_location != 'global')): | ||
self[setting_location[0]][setting_location[1]][setting] = value | ||
return | ||
|
||
raise KeyError( | ||
f"{setting} is not available as a setting for language: '{lang}'") |
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.
Can we detail why?
If it's a circular import issue is there a way we can restructure to avoid this?
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.
That's exactly what it is, that's for the duration of the feature branch until I figure out exactly where the line is.
It may be possible to reduce the scope of
internal
'sconfig
imports and avoid the circle.