refactor to separate getting info and creating config files

Author: sbillinge

news/userinfo.rst

@@ -0,0 +1,24 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* Refactor get_user_info to separate the tasks of getting the info from config files
+  and creating config files when they are missing.
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

src/diffpy/utils/tools.py

@@ -1,6 +1,5 @@
 import importlib.metadata
 import json
-import os
 from copy import copy
 from pathlib import Path
 
@@ -56,7 +55,7 @@ def load_config(file_path):
     Returns
     -------
     dict:
-        The configuration dictionary or None if file does not exist.
+        The configuration dictionary or {} if the config file does not exist.
 
     """
     config_file = Path(file_path).resolve()
@@ -65,7 +64,7 @@ def load_config(file_path):
             config = json.load(f)
         return config
     else:
-        return None
+        return {}
 
 
 def _sorted_merge(*dicts):
@@ -91,9 +90,9 @@ def _create_global_config(args):
     return return_bool
 
 
-def get_user_info(args=None):
+def get_user_info(owner_name=None, owner_email=None, owner_orcid=None):
     """
-    Get username and email configuration.
+    Get username, email and orcid configuration.
 
     First attempts to load config file from global and local paths.
     If neither exists, creates a global config file.
@@ -111,27 +110,26 @@ def get_user_info(args=None):
         The dictionary containing username and email with corresponding values.
 
     """
-    config_bool = True
+    runtime_info = {"owner_name": owner_name, "owner_email": owner_email, "owner_orcid": owner_orcid}
+    for key, value in copy(runtime_info).items():
+        if value is None or value == "":
+            del runtime_info[key]
     global_config = load_config(Path().home() / "diffpyconfig.json")
     local_config = load_config(Path().cwd() / "diffpyconfig.json")
-    if global_config is None and local_config is None:
-        print(
-            "No global configuration file was found containing "
-            "information about the user to associate with the data.\n"
-            "By following the prompts below you can add your name and email to this file on the current "
-            "computer and your name will be automatically associated with subsequent diffpy data by default.\n"
-            "This is not recommended on a shared or public computer. "
-            "You will only have to do that once.\n"
-            "For more information, please refer to www.diffpy.org/diffpy.utils/examples/toolsexample.html"
-        )
-        config_bool = _create_global_config(args)
-        global_config = load_config(Path().home() / "diffpyconfig.json")
-    config = _sorted_merge(clean_dict(global_config), clean_dict(local_config), clean_dict(args))
-    if config_bool is False:
-        os.remove(Path().home() / "diffpyconfig.json")
-        config = {"username": "", "email": ""}
-
-    return config
+    # if global_config is None and local_config is None:
+    #     print(
+    #         "No global configuration file was found containing "
+    #         "information about the user to associate with the data.\n"
+    #         "By following the prompts below you can add your name and email to this file on the current "
+    #         "computer and your name will be automatically associated with subsequent diffpy data by default.\n"
+    #         "This is not recommended on a shared or public computer. "
+    #         "You will only have to do that once.\n"
+    #         "For more information, please refer to www.diffpy.org/diffpy.utils/examples/toolsexample.html"
+    #     )
+    user_info = global_config
+    user_info.update(local_config)
+    user_info.update(runtime_info)
+    return user_info
 
 
 def get_package_info(package_names, metadata=None):

tests/conftest.py

@@ -12,14 +12,16 @@ def user_filesystem(tmp_path):
     base_dir = Path(tmp_path)
     home_dir = base_dir / "home_dir"
     home_dir.mkdir(parents=True, exist_ok=True)
-    cwd_dir = base_dir / "cwd_dir"
+    cwd_dir = home_dir / "cwd_dir"
     cwd_dir.mkdir(parents=True, exist_ok=True)
-
-    home_config_data = {"username": "home_username", "email": "[email protected]"}
+    home_config_data = {
+        "owner_name": "home_ownername",
+        "owner_email": "[email protected]",
+        "owner_orcid": "home_orcid",
+    }
     with open(home_dir / "diffpyconfig.json", "w") as f:
         json.dump(home_config_data, f)
-
-    yield tmp_path
+    yield home_dir, cwd_dir
 
 
 @pytest.fixture

tests/test_tools.py

@@ -8,11 +8,9 @@
 from diffpy.utils.tools import get_package_info, get_user_info
 
 
-def _setup_dirs(monkeypatch, user_filesystem):
-    cwd = Path(user_filesystem)
-    home_dir = cwd / "home_dir"
-    monkeypatch.setattr("pathlib.Path.home", lambda _: home_dir)
-    os.chdir(cwd)
+def _setup_dirs(user_filesystem):
+    home_dir, cwd_dir = user_filesystem.home_dir, user_filesystem.cwd_dir
+    os.chdir(cwd_dir)
     return home_dir
 
 
@@ -24,15 +22,6 @@ def _run_tests(inputs, expected):
     assert config.get("email") == expected_email
 
 
-params_user_info_with_home_conf_file = [
-    (["", ""], ["home_username", "[email protected]"]),
-    (["cli_username", ""], ["cli_username", "[email protected]"]),
-    (["", "[email protected]"], ["home_username", "[email protected]"]),
-    ([None, None], ["home_username", "[email protected]"]),
-    (["cli_username", None], ["cli_username", "[email protected]"]),
-    ([None, "[email protected]"], ["home_username", "[email protected]"]),
-    (["cli_username", "[email protected]"], ["cli_username", "[email protected]"]),
-]
 params_user_info_with_local_conf_file = [
     (["", ""], ["cwd_username", "[email protected]"]),
     (["cli_username", ""], ["cli_username", "[email protected]"]),
@@ -84,21 +73,80 @@ def _run_tests(inputs, expected):
 ]
 
 
-@pytest.mark.parametrize("inputs, expected", params_user_info_with_home_conf_file)
-def test_get_user_info_with_home_conf_file(monkeypatch, inputs, expected, user_filesystem):
-    _setup_dirs(monkeypatch, user_filesystem)
-    _run_tests(inputs, expected)
-
-
-@pytest.mark.parametrize("inputs, expected", params_user_info_with_local_conf_file)
-def test_get_user_info_with_local_conf_file(monkeypatch, inputs, expected, user_filesystem):
-    _setup_dirs(monkeypatch, user_filesystem)
-    local_config_data = {"username": "cwd_username", "email": "[email protected]"}
-    with open(Path(user_filesystem) / "diffpyconfig.json", "w") as f:
+@pytest.mark.parametrize(
+    "runtime_inputs, expected",
+    [  # config file in home is present, no config in cwd.  various runtime values passed
+        # C1: nothing passed in, expect uname, email, orcid from home_config
+        ({}, {"owner_name": "home_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"}),
+        # C2: empty strings passed in, expect uname, email, orcid from home_config
+        (
+            {"owner_name": "", "owner_email": "", "owner_orcid": ""},
+            {"owner_name": "home_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"},
+        ),
+        # C3: just owner name passed in at runtime.  expect runtime_oname but others from config
+        (
+            {"owner_name": "runtime_ownername"},
+            {"owner_name": "runtime_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"},
+        ),
+        # C4: just owner email passed in at runtime.  expect runtime_email but others from config
+        (
+            {"owner_email": "[email protected]"},
+            {"owner_name": "home_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"},
+        ),
+        # C5: just owner ci passed in at runtime.  expect runtime_orcid but others from config
+        (
+            {"owner_orcid": "runtime_orcid"},
+            {"owner_name": "home_ownername", "owner_email": "[email protected]", "owner_orcid": "runtime_orcid"},
+        ),
+    ],
+)
+def test_get_user_info_with_home_conf_file(runtime_inputs, expected, user_filesystem, mocker):
+    # user_filesystem[0] is tmp_dir/home_dir with the global config file in it, user_filesystem[1]
+    #   is tmp_dir/cwd_dir
+    mocker.patch.object(Path, "home", return_value=user_filesystem[0])
+    os.chdir(user_filesystem[1])
+    actual = get_user_info(**runtime_inputs)
+    assert actual == expected
+
+
+@pytest.mark.parametrize(
+    "runtime_inputs, expected",
+    [  # tests as before but now config file present in cwd and home but orcid
+        #   missing in the cwd config
+        # C1: nothing passed in, expect uname, email from local config, orcid from home_config
+        ({}, {"owner_name": "cwd_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"}),
+        # C2: empty strings passed in, expect uname, email, orcid from home_config
+        (
+            {"owner_name": "", "owner_email": "", "owner_orcid": ""},
+            {"owner_name": "cwd_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"},
+        ),
+        # C3: just owner name passed in at runtime.  expect runtime_oname but others from config
+        (
+            {"owner_name": "runtime_ownername"},
+            {"owner_name": "runtime_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"},
+        ),
+        # C4: just owner email passed in at runtime.  expect runtime_email but others from config
+        (
+            {"owner_email": "[email protected]"},
+            {"owner_name": "cwd_ownername", "owner_email": "[email protected]", "owner_orcid": "home_orcid"},
+        ),
+        # C5: just owner ci passed in at runtime.  expect runtime_orcid but others from config
+        (
+            {"owner_orcid": "runtime_orcid"},
+            {"owner_name": "cwd_ownername", "owner_email": "[email protected]", "owner_orcid": "runtime_orcid"},
+        ),
+    ],
+)
+def test_get_user_info_with_local_conf_file(runtime_inputs, expected, user_filesystem, mocker):
+    # user_filesystem[0] is tmp_dir/home_dir with the global config file in it, user_filesystem[1] i
+    #   s tmp_dir/cwd_dir
+    mocker.patch.object(Path, "home", return_value=user_filesystem[0])
+    os.chdir(user_filesystem[1])
+    local_config_data = {"owner_name": "cwd_ownername", "owner_email": "[email protected]"}
+    with open(user_filesystem[1] / "diffpyconfig.json", "w") as f:
         json.dump(local_config_data, f)
-    _run_tests(inputs, expected)
-    os.remove(Path().home() / "diffpyconfig.json")
-    _run_tests(inputs, expected)
+    actual = get_user_info(**runtime_inputs)
+    assert actual == expected
 
 
 @pytest.mark.parametrize("inputsa, inputsb, expected", params_user_info_with_no_home_conf_file)