Get rid of most global variables in configparser

tests/test_config.py

@@ -6,13 +6,15 @@ import pytest
 
 from theano import configdefaults, configparser
 from theano.configdefaults import default_blas_ldflags
-from theano.configparser import THEANO_FLAGS_DICT, AddConfigVar, ConfigParam
+from theano.configparser import ConfigParam
 
 
 def test_invalid_default():
     # Ensure an invalid default value found in the Theano code only causes
     # a crash if it is not overridden by the user.
 
+    root = configdefaults.config
+
     def validate(val):
         if val == "invalid":
             raise ValueError("Test-triggered")
@@ -20,17 +22,17 @@ def test_invalid_default():
     with pytest.raises(ValueError, match="Test-triggered"):
         # This should raise a ValueError because the default value is
         # invalid.
-        AddConfigVar(
+        root.add(
             "T_config__test_invalid_default_a",
             doc="unittest",
             configparam=ConfigParam("invalid", validate=validate),
             in_c_key=False,
         )
 
-    THEANO_FLAGS_DICT["T_config__test_invalid_default_b"] = "ok"
+    root._flags_dict["T_config__test_invalid_default_b"] = "ok"
     # This should succeed since we defined a proper value, even
     # though the default was invalid.
-    AddConfigVar(
+    root.add(
         "T_config__test_invalid_default_b",
         doc="unittest",
         configparam=ConfigParam("invalid", validate=validate),
@@ -39,7 +41,7 @@ def test_invalid_default():
 
     # TODO We should remove these dummy options on test exit.
     # Check that the flag has been removed
-    assert "T_config__test_invalid_default_b" not in THEANO_FLAGS_DICT
+    assert "T_config__test_invalid_default_b" not in root._flags_dict
 
 
 @patch("theano.configdefaults.try_blas_flag", return_value=None)
@@ -84,20 +86,19 @@ def test_config_param_apply_and_validation():
 def test_config_hash():
     # TODO: use custom config instance for the test
     root = configparser.config
-    configparser.AddConfigVar(
+    root.add(
         "test_config_hash",
         "A config var from a test case.",
         configparser.StrParam("test_default"),
-        root=root,
     )
 
-    h0 = configparser.get_config_hash()
+    h0 = root.get_config_hash()
 
     with configparser.change_flags(test_config_hash="new_value"):
         assert root.test_config_hash == "new_value"
-        h1 = configparser.get_config_hash()
+        h1 = root.get_config_hash()
 
-    h2 = configparser.get_config_hash()
+    h2 = root.get_config_hash()
     assert h1 != h0
     assert h2 == h0
 
@@ -141,11 +142,10 @@ class TestConfigTypes:
 def test_config_context():
     # TODO: use custom config instance for the test
     root = configparser.config
-    configparser.AddConfigVar(
+    root.add(
         "test_config_context",
         "A config var from a test case.",
         configparser.StrParam("test_default"),
-        root=root,
     )
     assert hasattr(root, "test_config_context")
     assert root.test_config_context == "test_default"
@@ -156,8 +156,9 @@ def test_config_context():
 
 
 def test_no_more_dotting():
+    root = configparser.config
     with pytest.raises(ValueError, match="Dot-based"):
-        AddConfigVar(
+        root.add(
             "T_config.something",
             doc="unittest",
             configparam=ConfigParam("invalid"),

theano/configdefaults.py

@@ -12,8 +12,8 @@ import warnings
 import numpy as np
 
 import theano
+import theano.configparser
 from theano.configparser import (
-    THEANO_FLAGS_DICT,
     AddConfigVar,
     BoolParam,
     ConfigParam,
@@ -23,7 +23,6 @@ from theano.configparser import (
     FloatParam,
     IntParam,
     StrParam,
-    TheanoConfigParser,
 )
 from theano.misc.windows import call_subprocess_Popen, output_subprocess_Popen
 from theano.utils import maybe_add_to_os_environ_pathlist
@@ -2236,7 +2235,7 @@ SUPPORTED_DNN_CONV_PRECISION = (
 # TODO: we should add the configs explicitly to an instance of the TheanoConfigParser
 # Even if we treat it as a singleton, we should implement as if it wasn't, so we can
 # test it independently.
-config = TheanoConfigParser()
+config = theano.configparser.config
 add_basic_configvars()
 add_dnn_configvars()
 add_magma_configvars()
@@ -2272,6 +2271,5 @@ except OSError:
 add_caching_dir_configvars()
 
 # Check if there are remaining flags provided by the user through THEANO_FLAGS.
-# TODO: the global variable THEANO_FLAGS_DICT should probably become an attribute on the `config` singleton
-for key in THEANO_FLAGS_DICT.keys():
+for key in config._flags_dict.keys():
     warnings.warn(f"Theano does not recognise this flag: {key}")

theano/configparser.py

@@ -20,64 +20,12 @@ class TheanoConfigWarning(Warning):
     warn = classmethod(warn)
 
 
-def parse_config_string(config_string, issue_warnings=True):
-    """
-    Parses a config string (comma-separated key=value components) into a dict.
-    """
-    config_dict = {}
-    my_splitter = shlex.shlex(config_string, posix=True)
-    my_splitter.whitespace = ","
-    my_splitter.whitespace_split = True
-    for kv_pair in my_splitter:
-        kv_pair = kv_pair.strip()
-        if not kv_pair:
-            continue
-        kv_tuple = kv_pair.split("=", 1)
-        if len(kv_tuple) == 1:
-            if issue_warnings:
-                TheanoConfigWarning.warn(
-                    f"Config key '{kv_tuple[0]}' has no value, ignoring it",
-                    stacklevel=1,
-                )
-        else:
-            k, v = kv_tuple
-            # subsequent values for k will override earlier ones
-            config_dict[k] = v
-    return config_dict
-
-
-# THEANORC can contain a colon-delimited list of config files, like
-# THEANORC=~lisa/.theanorc:~/.theanorc
-# In that case, definitions in files on the right (here, ~/.theanorc) have
-# precedence over those in files on the left.
-def config_files_from_theanorc():
-    rval = [
-        os.path.expanduser(s)
-        for s in os.getenv("THEANORC", "~/.theanorc").split(os.pathsep)
-    ]
-    if os.getenv("THEANORC") is None and sys.platform == "win32":
-        # to don't need to change the filename and make it open easily
-        rval.append(os.path.expanduser("~/.theanorc.txt"))
-    return rval
-
-
-class change_flags:
-    """
-    Use this as a decorator or context manager to change the value of
-    Theano config variables.
-
-    Useful during tests.
-    """
-
-    def __init__(self, args=(), **kwargs):
-        confs = dict()
+class _ChangeFlagsDecorator:
+    def __init__(self, *args, _root=None, **kwargs):
+        # the old API supported passing a dict as the first argument:
         args = dict(args)
         args.update(kwargs)
-        for k in args:
-            l = [v for v in _config_var_list if v.fullname == k]
-            assert len(l) == 1, l
-            confs[k] = l[0]
-        self.confs = confs
+        self.confs = {k: _root._config_var_dict[k] for k in args}
         self.new_vals = args
 
     def __call__(self, f):
@@ -105,7 +53,116 @@ class change_flags:
             v.__set__(None, self.old_vals[k])
 
 
-def fetch_val_for_key(key, delete_key=False):
+def _hash_from_code(msg):
+    """This function was copied from theano.gof.utils to get rid of that import."""
+    # hashlib.sha256() requires an object that supports buffer interface,
+    # but Python 3 (unicode) strings don't.
+    if isinstance(msg, str):
+        msg = msg.encode()
+    # Python 3 does not like module names that start with
+    # a digit.
+    return "m" + hashlib.sha256(msg).hexdigest()
+
+
+class TheanoConfigParser:
+    """ Object that holds configuration settings. """
+
+    def __init__(self, flags_dict: dict, theano_cfg, theano_raw_cfg):
+        self._flags_dict = flags_dict
+        self._theano_cfg = theano_cfg
+        self._theano_raw_cfg = theano_raw_cfg
+        self._config_var_dict = {}
+        super().__init__()
+
+    def __str__(self, print_doc=True):
+        sio = StringIO()
+        self.config_print(buf=sio, print_doc=print_doc)
+        return sio.getvalue()
+
+    def config_print(self, buf, print_doc=True):
+        for cv in self._config_var_dict.values():
+            print(cv, file=buf)
+            if print_doc:
+                print("    Doc: ", cv.doc, file=buf)
+            print("    Value: ", cv.__get__(True, None), file=buf)
+            print("", file=buf)
+
+    def get_config_hash(self):
+        """
+        Return a string sha256 of the current config options. In the past,
+        it was md5.
+
+        The string should be such that we can safely assume that two different
+        config setups will lead to two different strings.
+
+        We only take into account config options for which `in_c_key` is True.
+        """
+        all_opts = sorted(
+            [c for c in self._config_var_dict.values() if c.in_c_key],
+            key=lambda cv: cv.fullname,
+        )
+        return _hash_from_code(
+            "\n".join(
+                [
+                    "{} = {}".format(cv.fullname, cv.__get__(True, None))
+                    for cv in all_opts
+                ]
+            )
+        )
+
+    def add(self, name, doc, configparam, in_c_key=True):
+        """Add a new variable to TheanoConfigParser.
+
+        This method performs some of the work of initializing `ConfigParam` instances.
+
+        Parameters
+        ----------
+        name: string
+            The full name for this configuration variable. Takes the form
+            ``"[section0__[section1__[etc]]]_option"``.
+        doc: string
+            A string that provides documentation for the config variable.
+        configparam: ConfigParam
+            An object for getting and setting this configuration parameter
+        in_c_key: boolean
+            If ``True``, then whenever this config option changes, the key
+            associated to compiled C modules also changes, i.e. it may trigger a
+            compilation of these modules (this compilation will only be partial if it
+            turns out that the generated C code is unchanged). Set this option to False
+            only if you are confident this option should not affect C code compilation.
+
+        """
+        if "." in name:
+            raise ValueError(
+                f"Dot-based sections were removed. Use double underscores! ({name})"
+            )
+        if hasattr(self, name):
+            raise AttributeError(f"The name {name} is already taken")
+        configparam.doc = doc
+        configparam.fullname = name
+        configparam.in_c_key = in_c_key
+        # Trigger a read of the value from config files and env vars
+        # This allow to filter wrong value from the user.
+        if not callable(configparam.default):
+            configparam.__get__(self, type(self), delete_key=True)
+        else:
+            # We do not want to evaluate now the default value
+            # when it is a callable.
+            try:
+                self.fetch_val_for_key(name)
+                # The user provided a value, filter it now.
+                configparam.__get__(self, type(self), delete_key=True)
+            except KeyError:
+                _logger.error(
+                    f"Suppressed KeyError in AddConfigVar for parameter '{name}'!"
+                )
+
+        # the ConfigParam implements __get__/__set__, enabling us to create a property:
+        setattr(self.__class__, name, configparam)
+        # keep the ConfigParam object in a dictionary:
+        self._config_var_dict[name] = configparam
+
+    def fetch_val_for_key(self, key, delete_key=False):
         """Return the overriding config value for a key.
         A successful search returns a string value.
         An unsuccessful search raises a KeyError
@@ -117,10 +174,10 @@ def fetch_val_for_key(key, delete_key=False):
         """
 
         # first try to find it in the FLAGS
-    if key in THEANO_FLAGS_DICT:
+        if key in self._flags_dict:
             if delete_key:
-            return THEANO_FLAGS_DICT.pop(key)
-        return THEANO_FLAGS_DICT[key]
+                return self._flags_dict.pop(key)
+            return self._flags_dict[key]
 
         # next try to find it in the config file
 
@@ -135,61 +192,20 @@ def fetch_val_for_key(key, delete_key=False):
             section, option = "global", key
         try:
             try:
-            return theano_cfg.get(section, option)
+                return self._theano_cfg.get(section, option)
             except ConfigParser.InterpolationError:
-            return theano_raw_cfg.get(section, option)
+                return self._theano_raw_cfg.get(section, option)
         except (ConfigParser.NoOptionError, ConfigParser.NoSectionError):
             raise KeyError(key)
 
-
-def _config_print(thing, buf, print_doc=True):
-    for cv in _config_var_list:
-        print(cv, file=buf)
-        if print_doc:
-            print("    Doc: ", cv.doc, file=buf)
-        print("    Value: ", cv.__get__(True, None), file=buf)
-        print("", file=buf)
-
-
-def _hash_from_code(msg):
-    """This function was copied from theano.gof.utils to get rid of that import."""
-    # hashlib.sha256() requires an object that supports buffer interface,
-    # but Python 3 (unicode) strings don't.
-    if isinstance(msg, str):
-        msg = msg.encode()
-    # Python 3 does not like module names that start with
-    # a digit.
-    return "m" + hashlib.sha256(msg).hexdigest()
-
-
-def get_config_hash():
+    def change_flags(self, *args, **kwargs) -> _ChangeFlagsDecorator:
         """
-    Return a string sha256 of the current config options. In the past,
-    it was md5.
-
-    The string should be such that we can safely assume that two different
-    config setups will lead to two different strings.
+        Use this as a decorator or context manager to change the value of
+        Theano config variables.
 
-    We only take into account config options for which `in_c_key` is True.
+        Useful during tests.
         """
-    all_opts = sorted(
-        [c for c in _config_var_list if c.in_c_key], key=lambda cv: cv.fullname
-    )
-    return _hash_from_code(
-        "\n".join(
-            ["{} = {}".format(cv.fullname, cv.__get__(True, None)) for cv in all_opts]
-        )
-    )
-
-
-class TheanoConfigParser:
-    # properties are installed by AddConfigVar
-    _i_am_a_config_class = True
-
-    def __str__(self, print_doc=True):
-        sio = StringIO()
-        _config_print(self.__class__, sio, print_doc=print_doc)
-        return sio.getvalue()
+        return _ChangeFlagsDecorator(*args, _root=self, **kwargs)
 
 
 class ConfigParam:
@@ -198,6 +214,9 @@ class ConfigParam:
     A ConfigParam has not only default values and configurable mutability, but
     also documentation text, as well as filtering and validation routines
     that can be context-dependent.
+
+    This class implements __get__ and __set__ methods to eventually become
+    a property on an instance of TheanoConfigParser.
     """
 
     def __init__(
@@ -230,9 +249,10 @@ class ConfigParam:
         self._validate = validate
         self._mutable = mutable
         self.is_default = True
-        # set by AddConfigVar:
+        # set by TheanoConfigParser.add:
         self.fullname = None
         self.doc = None
+        self.in_c_key = None
 
         # Note that we do not call `self.filter` on the default value: this
         # will be done automatically in AddConfigVar, potentially with a
@@ -279,7 +299,7 @@ class ConfigParam:
             return self
         if not hasattr(self, "val"):
             try:
-                val_str = fetch_val_for_key(self.fullname, delete_key=delete_key)
+                val_str = cls.fetch_val_for_key(self.fullname, delete_key=delete_key)
                 self.is_default = False
             except KeyError:
                 if callable(self.default):
@@ -421,16 +441,58 @@ class ContextsParam(ConfigParam):
         return val
 
 
-# TODO: Not all of the following variables need to exist. Most should be private.
-THEANO_FLAGS = os.getenv("THEANO_FLAGS", "")
-# The THEANO_FLAGS environment variable should be a list of comma-separated
-# [section__]option=value entries. If the section part is omitted, there should
-# be only one section that contains the given option.
-THEANO_FLAGS_DICT = parse_config_string(THEANO_FLAGS, issue_warnings=True)
-_config_var_list = []
+def parse_config_string(config_string, issue_warnings=True):
+    """
+    Parses a config string (comma-separated key=value components) into a dict.
+    """
+    config_dict = {}
+    my_splitter = shlex.shlex(config_string, posix=True)
+    my_splitter.whitespace = ","
+    my_splitter.whitespace_split = True
+    for kv_pair in my_splitter:
+        kv_pair = kv_pair.strip()
+        if not kv_pair:
+            continue
+        kv_tuple = kv_pair.split("=", 1)
+        if len(kv_tuple) == 1:
+            if issue_warnings:
+                TheanoConfigWarning.warn(
+                    f"Config key '{kv_tuple[0]}' has no value, ignoring it",
+                    stacklevel=1,
+                )
+        else:
+            k, v = kv_tuple
+            # subsequent values for k will override earlier ones
+            config_dict[k] = v
+    return config_dict
+
+
+def config_files_from_theanorc():
+    """
+    THEANORC can contain a colon-delimited list of config files, like
+    THEANORC=~lisa/.theanorc:~/.theanorc
+    In that case, definitions in files on the right (here, ~/.theanorc) have
+    precedence over those in files on the left.
+    """
+    rval = [
+        os.path.expanduser(s)
+        for s in os.getenv("THEANORC", "~/.theanorc").split(os.pathsep)
+    ]
+    if os.getenv("THEANORC") is None and sys.platform == "win32":
+        # to don't need to change the filename and make it open easily
+        rval.append(os.path.expanduser("~/.theanorc.txt"))
+    return rval
+
+
+def _create_default_config():
+    # The THEANO_FLAGS environment variable should be a list of comma-separated
+    # [section__]option=value entries. If the section part is omitted, there should
+    # be only one section that contains the given option.
+    THEANO_FLAGS = os.getenv("THEANO_FLAGS", "")
+    THEANO_FLAGS_DICT = parse_config_string(THEANO_FLAGS, issue_warnings=True)
 
-config_files = config_files_from_theanorc()
-theano_cfg = ConfigParser.ConfigParser(
+    config_files = config_files_from_theanorc()
+    theano_cfg = ConfigParser.ConfigParser(
         {
             "USER": os.getenv("USER", os.path.split(os.path.expanduser("~"))[-1]),
             "LSCRATCH": os.getenv("LSCRATCH", ""),
@@ -439,85 +501,26 @@ theano_cfg = ConfigParser.ConfigParser(
             "TMP": os.getenv("TMP", ""),
             "PID": str(os.getpid()),
         }
-)
-theano_cfg.read(config_files)
-# Having a raw version of the config around as well enables us to pass
-# through config values that contain format strings.
-# The time required to parse the config twice is negligible.
-theano_raw_cfg = ConfigParser.RawConfigParser()
-theano_raw_cfg.read(config_files)
-
-# N.B. all instances of TheanoConfigParser give access to the same properties.
-config = TheanoConfigParser()
-
-
-def AddConfigVar(name, doc, configparam, root=config, in_c_key=True):
-    """Add a new variable to `theano.config`.
-
-    The data structure at work here is a tree of classes with class
-    attributes/properties that are either a) instantiated dynamically-generated
-    classes, or b) `ConfigParam` instances.  The root of this tree is the
-    `TheanoConfigParser` class, and the internal nodes are the ``SubObj``
-    classes created inside of `AddConfigVar`.
-
-    Why this design?
-    - The config object is a true singleton.  Every instance of
-    `TheanoConfigParser` is an empty instance that looks up
-    attributes/properties in the [single] ``TheanoConfigParser.__dict__``
-    - The subtrees provide the same interface as the root
-    - `ConfigParser` subclasses control get/set of config properties to guard
-    against craziness.
-
-    This method also performs some of the work of initializing `ConfigParam`
-    instances
-
-    Parameters
-    ----------
-    name: string
-        The full name for this configuration variable. Takes the form
-        ``"[section0__[section1__[etc]]]_option"``.
-    doc: string
-        A string that provides documentation for the config variable.
-    configparam: ConfigParam
-        An object for getting and setting this configuration parameter
-    root: object
-        Used for recursive calls.  Do not provide a value for this parameter.
-    in_c_key: boolean
-        If ``True``, then whenever this config option changes, the key
-        associated to compiled C modules also changes, i.e. it may trigger a
-        compilation of these modules (this compilation will only be partial if it
-        turns out that the generated C code is unchanged). Set this option to False
-        only if you are confident this option should not affect C code compilation.
-
-    """
-
-    if root is config:
-        # Only set the name in the first call, not the recursive ones
-        configparam.fullname = name
-    if "." in name:
-        raise ValueError(
-            f"Dot-based sections were removed. Use double underscores! ({name})"
     )
-    if hasattr(root, name):
-        raise AttributeError(f"The name {configparam.fullname} is already taken")
-    configparam.doc = doc
-    configparam.in_c_key = in_c_key
-    # Trigger a read of the value from config files and env vars
-    # This allow to filter wrong value from the user.
-    if not callable(configparam.default):
-        configparam.__get__(root, type(root), delete_key=True)
-    else:
-        # We do not want to evaluate now the default value
-        # when it is a callable.
-        try:
-            fetch_val_for_key(configparam.fullname)
-            # The user provided a value, filter it now.
-            configparam.__get__(root, type(root), delete_key=True)
-        except KeyError:
-            _logger.error(
-                f"Suppressed KeyError in AddConfigVar for parameter '{name}' with fullname '{configparam.fullname}'!"
+    theano_cfg.read(config_files)
+    # Having a raw version of the config around as well enables us to pass
+    # through config values that contain format strings.
+    # The time required to parse the config twice is negligible.
+    theano_raw_cfg = ConfigParser.RawConfigParser()
+    theano_raw_cfg.read(config_files)
+
+    # Instances of TheanoConfigParser can have independent current values!
+    # But because the properties are assinged to the type, their existence is global.
+    config = TheanoConfigParser(
+        flags_dict=THEANO_FLAGS_DICT,
+        theano_cfg=theano_cfg,
+        theano_raw_cfg=theano_raw_cfg,
     )
-    setattr(root.__class__, name, configparam)
-    # TODO: After assigning the configvar to the "root" object, there should be
-    # no reason to keep the _config_var_list around!
-    _config_var_list.append(configparam)
+    return config
+
+
+config = _create_default_config()
+# aliasing for old API
+AddConfigVar = config.add
+change_flags = config.change_flags
+_config_print = config.config_print

theano/gof/cc.py

@@ -11,7 +11,6 @@ from io import StringIO
 
 import numpy as np
 
-import theano
 from theano import config
 from theano.gof import cmodule, graph, link, utils
 from theano.gof.callcache import CallCache
@@ -1472,7 +1471,7 @@ class CLinker(link.Linker):
         # NOTE: config md5 is not using md5 hash, but sha256 instead. Function
         # string instances of md5 will be updated at a later release.
         if insert_config_hash:
-            sig.append("md5:" + theano.configparser.get_config_hash())
+            sig.append("md5:" + config.get_config_hash())
         else:
             sig.append("md5: <omitted>")
 

theano/gof/vm.py

@@ -13,7 +13,7 @@ import warnings
 from collections import defaultdict
 
 import theano.gof.cmodule
-from theano.configparser import _config_var_list, config
+from theano.configparser import config
 
 from . import link
 
@@ -715,9 +715,7 @@ except (OSError, theano.gof.cmodule.MissingGXX) as e:
     # already changed the default linker to something else then CVM.
     # Currently this is the py linker.
     # Here we assert that the default linker is not cvm.
-    assert not [x for x in _config_var_list if x.fullname == "linker"][
-        0
-    ].default.startswith("cvm"), e
+    assert not config._config_var_dict["linker"].default.startswith("cvm"), e
 
 
 class VM_Linker(link.LocalLinker):