From f064217187ece84eb10dd57e130112cf3ea19cef Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Sun, 22 Dec 2024 07:13:13 -0500 Subject: [PATCH 1/5] initial commit of tests for the config updater. I need to get the updated user_filesystem fixture so merging main next --- news/configupdate.rst | 23 +++++++++++++++++++++++ src/diffpy/utils/tools.py | 5 ++++- tests/test_tools.py | 32 +++++++++++++++++++++++++++++++- 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 news/configupdate.rst diff --git a/news/configupdate.rst b/news/configupdate.rst new file mode 100644 index 00000000..0bc3ffa9 --- /dev/null +++ b/news/configupdate.rst @@ -0,0 +1,23 @@ +**Added:** + +* no news added: covered in the news from the get_user_info work + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/utils/tools.py b/src/diffpy/utils/tools.py index 3fc10031..98db017e 100644 --- a/src/diffpy/utils/tools.py +++ b/src/diffpy/utils/tools.py @@ -130,10 +130,13 @@ def get_user_info(args=None): if config_bool is False: os.remove(Path().home() / "diffpyconfig.json") config = {"username": "", "email": ""} - return config +def check_and_build_global_config(): + return + + def get_package_info(package_names, metadata=None): """ Fetches package version and updates it into (given) metadata. diff --git a/tests/test_tools.py b/tests/test_tools.py index 0a42332f..117a8565 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -5,7 +5,7 @@ import pytest -from diffpy.utils.tools import get_package_info, get_user_info +from diffpy.utils.tools import check_and_build_global_config, get_package_info, get_user_info def _setup_dirs(monkeypatch, user_filesystem): @@ -123,6 +123,36 @@ def test_get_user_info_no_conf_file_no_inputs(monkeypatch, inputsa, inputsb, exp assert confile.exists() is False +@pytest.mark.parametrize( + "test_inputs,expected", + [ # Check check_and_build_global_config() builds correct config when config is found missing + ( # C1: user inputs valid name, email and orcid + {"user_inputs": ["input_name", "input@email.com", "input_orcid"]}, + {"owner_email": "input@email.com", "owner_orcid": "input_orcid", "owner_name": "input_name"}, + ), + # ( # C2: empty strings passed in, expect uname, email, orcid from home_config + # {"owner_name": "", "owner_email": "", "owner_orcid": ""}, + # {"owner_name": "home_ownername", "owner_email": "home@email.com", "owner_orcid": "home_orcid"}, + # ), + ], +) +def test_check_and_build_global_config(test_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]) + # remove the config file from home that came with user_filesystem + old_confile = user_filesystem[0] / "diffpyconfig.json" + os.remove(old_confile) + check_and_build_global_config() + inp_iter = iter(test_inputs["user_inputs"]) + mocker.patch("builtins.input", lambda _: next(inp_iter)) + with open(old_confile, "r") as f: + actual = json.load(f) + print(actual) + assert actual == expected + + params_package_info = [ (["diffpy.utils", None], {"package_info": {"diffpy.utils": "3.3.0"}}), (["package1", None], {"package_info": {"package1": "1.2.3", "diffpy.utils": "3.3.0"}}), From ad05e37e7c0c9707d6270ff6c1c77734aee913eb Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Sun, 22 Dec 2024 09:18:48 -0500 Subject: [PATCH 2/5] check_and_build_global_config added wih tests --- src/diffpy/utils/tools.py | 70 +++++++++++++++++++++++---------------- tests/test_tools.py | 66 ++++++++++++++++++------------------ 2 files changed, 74 insertions(+), 62 deletions(-) diff --git a/src/diffpy/utils/tools.py b/src/diffpy/utils/tools.py index 1fd0ed70..59c5d31d 100644 --- a/src/diffpy/utils/tools.py +++ b/src/diffpy/utils/tools.py @@ -74,22 +74,6 @@ def _sorted_merge(*dicts): return merged -def _create_global_config(args): - username = input( - f"Please enter the name you would want future work to be credited to " f"[{args.get('username', '')}]: " - ).strip() or args.get("username", "") - email = input(f"Please enter the your email " f"[{args.get('email', '')}]: ").strip() or args.get("email", "") - return_bool = False if username is None or email is None else True - with open(Path().home() / "diffpyconfig.json", "w") as f: - f.write(json.dumps({"username": stringify(username), "email": stringify(email)})) - print( - f"You can manually edit the config file at {Path().home() / 'diffpyconfig.json'} using any text editor.\n" - f"Or you can update the config file by passing new values to get_user_info(), " - f"see examples here: https://www.diffpy.org/diffpy.utils/examples/toolsexample.html" - ) - return return_bool - - def get_user_info(owner_name=None, owner_email=None, owner_orcid=None): """ Get name, email and orcid of the owner/user from various sources and return it as a metadata dictionary @@ -107,7 +91,7 @@ def get_user_info(owner_name=None, owner_email=None, owner_orcid=None): "owner_email": ">@email.com", "owner_orcid": ">" } - You may also store any other gloabl-level information that you would like associated with your + You may also store any other global-level information that you would like associated with your diffraction data in this file Parameters @@ -132,24 +116,52 @@ def get_user_info(owner_name=None, owner_email=None, owner_orcid=None): 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" - # ) user_info = global_config user_info.update(local_config) user_info.update(runtime_info) return user_info -def check_and_build_global_config(): - return +def _get_value(mystring): + return mystring.strip() + + +def check_and_build_global_config(skip_config_creation=False): + config_path = Path().home() / "diffpyconfig.json" + if skip_config_creation: + return + if config_path.is_file(): + return + intro_text = ( + "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" + ) + print(intro_text) + username = input("Please enter the name you would want future work to be credited to: ").strip() + email = input("Please enter your email: ").strip() + orcid = input("Please enter your orcid ID if you know it: ").strip() + config = {"owner_name": stringify(username), "owner_email": stringify(email), "owner_orcid": stringify(orcid)} + if email != "" or orcid != "" or username != "": + config["owner_orcid"] = stringify(orcid) + with open(config_path, "w") as f: + f.write(json.dumps(config)) + outro_text = ( + f"The config file at {Path().home() / 'diffpyconfig.json'} has been created. " + f"The values {config} were entered.\n" + f"These values will be inserted as metadata with your data in apps that use " + f"diffpy.get_user_info(). If you would like to update these values, either " + f"delete the config file and this workflow will rerun next time you run this " + f"program. Or you may open the config file in a text editor and manually edit the" + f"entries. For more information, see: " + f"https://diffpy.githu.io/diffpy.utils/examples/tools_example.html" + ) + print(outro_text) + return def get_package_info(package_names, metadata=None): diff --git a/tests/test_tools.py b/tests/test_tools.py index 3169f01f..88e81566 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -149,29 +149,6 @@ def test_get_user_info_with_local_conf_file(runtime_inputs, expected, user_files assert actual == expected -# @pytest.mark.parametrize("inputsa, inputsb, expected", params_user_info_with_no_home_conf_file) -# def test_get_user_info_with_no_home_conf_file(monkeypatch, inputsa, inputsb, expected, user_filesystem): -# _setup_dirs(monkeypatch, user_filesystem) -# os.remove(Path().home() / "diffpyconfig.json") -# inp_iter = iter(inputsb) -# monkeypatch.setattr("builtins.input", lambda _: next(inp_iter)) -# _run_tests(inputsa, expected) -# confile = Path().home() / "diffpyconfig.json" -# assert confile.is_file() -# -# -# @pytest.mark.parametrize("inputsa, inputsb, expected", params_user_info_no_conf_file_no_inputs) -# def test_get_user_info_no_conf_file_no_inputs(monkeypatch, inputsa, inputsb, expected, user_filesystem): -# _setup_dirs(monkeypatch, user_filesystem) -# os.remove(Path().home() / "diffpyconfig.json") -# inp_iter = iter(inputsb) -# monkeypatch.setattr("builtins.input", lambda _: next(inp_iter)) -# _run_tests(inputsa, expected) -# confile = Path().home() / "diffpyconfig.json" -# assert confile.exists() is False -# - - @pytest.mark.parametrize( "test_inputs,expected", [ # Check check_and_build_global_config() builds correct config when config is found missing @@ -179,10 +156,11 @@ def test_get_user_info_with_local_conf_file(runtime_inputs, expected, user_files {"user_inputs": ["input_name", "input@email.com", "input_orcid"]}, {"owner_email": "input@email.com", "owner_orcid": "input_orcid", "owner_name": "input_name"}, ), - # ( # C2: empty strings passed in, expect uname, email, orcid from home_config - # {"owner_name": "", "owner_email": "", "owner_orcid": ""}, - # {"owner_name": "home_ownername", "owner_email": "home@email.com", "owner_orcid": "home_orcid"}, - # ), + ({"user_inputs": ["", "", ""]}, None), # C2: empty strings passed in, expect no config file created + ( # C3: just username input, expect config file but with some empty values + {"user_inputs": ["input_name", "", ""]}, + {"owner_email": "", "owner_orcid": "", "owner_name": "input_name"}, + ), ], ) def test_check_and_build_global_config(test_inputs, expected, user_filesystem, mocker): @@ -190,18 +168,40 @@ def test_check_and_build_global_config(test_inputs, expected, user_filesystem, m # is tmp_dir/cwd_dir mocker.patch.object(Path, "home", return_value=user_filesystem[0]) os.chdir(user_filesystem[1]) + confile = user_filesystem[0] / "diffpyconfig.json" # remove the config file from home that came with user_filesystem - old_confile = user_filesystem[0] / "diffpyconfig.json" - os.remove(old_confile) + os.remove(confile) + mocker.patch("builtins.input", side_effect=test_inputs["user_inputs"]) check_and_build_global_config() - inp_iter = iter(test_inputs["user_inputs"]) - mocker.patch("builtins.input", lambda _: next(inp_iter)) - with open(old_confile, "r") as f: + try: + with open(confile, "r") as f: + actual = json.load(f) + except FileNotFoundError: + actual = None + assert actual == expected + + +def test_check_and_build_global_config_file_exists(user_filesystem, mocker): + mocker.patch.object(Path, "home", return_value=user_filesystem[0]) + os.chdir(user_filesystem[1]) + confile = user_filesystem[0] / "diffpyconfig.json" + expected = {"owner_name": "home_ownername", "owner_email": "home@email.com", "owner_orcid": "home_orcid"} + check_and_build_global_config() + with open(confile, "r") as f: actual = json.load(f) - print(actual) assert actual == expected +def test_check_and_build_global_config_skipped(user_filesystem, mocker): + mocker.patch.object(Path, "home", return_value=user_filesystem[0]) + os.chdir(user_filesystem[1]) + confile = user_filesystem[0] / "diffpyconfig.json" + # remove the config file from home that came with user_filesystem + os.remove(confile) + check_and_build_global_config(skip_config_creation=True) + assert not confile.exists() + + params_package_info = [ (["diffpy.utils", None], {"package_info": {"diffpy.utils": "3.3.0"}}), (["package1", None], {"package_info": {"package1": "1.2.3", "diffpy.utils": "3.3.0"}}), From 552dd32f8b516577e6dfd3040a26b242c56c0eed Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Sun, 22 Dec 2024 09:25:15 -0500 Subject: [PATCH 3/5] delete unneeded _get_value() function and unneeded test parms --- src/diffpy/utils/tools.py | 4 --- tests/test_tools.py | 65 --------------------------------------- 2 files changed, 69 deletions(-) diff --git a/src/diffpy/utils/tools.py b/src/diffpy/utils/tools.py index 59c5d31d..27e49ff8 100644 --- a/src/diffpy/utils/tools.py +++ b/src/diffpy/utils/tools.py @@ -122,10 +122,6 @@ def get_user_info(owner_name=None, owner_email=None, owner_orcid=None): return user_info -def _get_value(mystring): - return mystring.strip() - - def check_and_build_global_config(skip_config_creation=False): config_path = Path().home() / "diffpyconfig.json" if skip_config_creation: diff --git a/tests/test_tools.py b/tests/test_tools.py index 88e81566..d364feb9 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -7,71 +7,6 @@ from diffpy.utils.tools import check_and_build_global_config, get_package_info, get_user_info -# def _setup_dirs(monkeypatch, user_filesystem): -# home_dir, cwd_dir = user_filesystem.home_dir, user_filesystem.cwd_dir -# os.chdir(cwd_dir) -# return home_dir -# - - -def _run_tests(inputs, expected): - args = {"username": inputs[0], "email": inputs[1]} - expected_username, expected_email = expected - config = get_user_info(args) - assert config.get("username") == expected_username - assert config.get("email") == expected_email - - -params_user_info_with_local_conf_file = [ - (["", ""], ["cwd_username", "cwd@email.com"]), - (["cli_username", ""], ["cli_username", "cwd@email.com"]), - (["", "cli@email.com"], ["cwd_username", "cli@email.com"]), - ([None, None], ["cwd_username", "cwd@email.com"]), - (["cli_username", None], ["cli_username", "cwd@email.com"]), - ([None, "cli@email.com"], ["cwd_username", "cli@email.com"]), - (["cli_username", "cli@email.com"], ["cli_username", "cli@email.com"]), -] -params_user_info_with_no_home_conf_file = [ - ( - [None, None], - ["input_username", "input@email.com"], - ["input_username", "input@email.com"], - ), - ( - ["cli_username", None], - ["", "input@email.com"], - ["cli_username", "input@email.com"], - ), - ( - [None, "cli@email.com"], - ["input_username", ""], - ["input_username", "cli@email.com"], - ), - ( - ["", ""], - ["input_username", "input@email.com"], - ["input_username", "input@email.com"], - ), - ( - ["cli_username", ""], - ["", "input@email.com"], - ["cli_username", "input@email.com"], - ), - ( - ["", "cli@email.com"], - ["input_username", ""], - ["input_username", "cli@email.com"], - ), - ( - ["cli_username", "cli@email.com"], - ["input_username", "input@email.com"], - ["cli_username", "cli@email.com"], - ), -] -params_user_info_no_conf_file_no_inputs = [ - ([None, None], ["", ""], ["", ""]), -] - @pytest.mark.parametrize( "runtime_inputs, expected", From f498a0740d26627175ffa8c073311ca3db0c2969 Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Wed, 25 Dec 2024 07:18:22 -0500 Subject: [PATCH 4/5] docstring for create global config and have it return a bool --- src/diffpy/utils/tools.py | 40 ++++++++++++++++++++++++++++++++++----- tests/test_tools.py | 11 ++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/diffpy/utils/tools.py b/src/diffpy/utils/tools.py index 27e49ff8..e7d44d12 100644 --- a/src/diffpy/utils/tools.py +++ b/src/diffpy/utils/tools.py @@ -123,11 +123,40 @@ def get_user_info(owner_name=None, owner_email=None, owner_orcid=None): def check_and_build_global_config(skip_config_creation=False): + """ + Checks for a global diffpu config file in user's home directory and creates one if it is missing + + The file it looks for is called diffpyconfig.json. This can contain anything in json format, but + minimally contains information about the computer owner. The information is used + when diffpy objects are created and saved to files or databases to retain ownership information + of datasets. For example, it is used by diffpy.utils.tools.get_user_info(). + + If the function finds no config file in the user's home directory it interrupts execution + and prompts the user for name, email, and orcid information. It then creates the config file + with this information inside it. + + The function returns True if the file exists and False otherwise. + + If you would like to check for a file but not run the file creation workflow you can set + the optional argument skip_config_creation to True. + + Parameters + ---------- + skip_config_creation: bool, optional, Default is False + The bool that will override the creation workflow even if no config file exists. + + Returns + ------- + bool: True if the file exists and False otherwise. + + """ + config_exists = False config_path = Path().home() / "diffpyconfig.json" - if skip_config_creation: - return if config_path.is_file(): - return + config_exists = True + return config_exists + if skip_config_creation: + return config_exists intro_text = ( "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 " @@ -154,10 +183,11 @@ def check_and_build_global_config(skip_config_creation=False): f"delete the config file and this workflow will rerun next time you run this " f"program. Or you may open the config file in a text editor and manually edit the" f"entries. For more information, see: " - f"https://diffpy.githu.io/diffpy.utils/examples/tools_example.html" + f"https://diffpy.github.io/diffpy.utils/examples/tools_example.html" ) print(outro_text) - return + config_exists = True + return config_exists def get_package_info(package_names, metadata=None): diff --git a/tests/test_tools.py b/tests/test_tools.py index d364feb9..3808537d 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -107,13 +107,16 @@ def test_check_and_build_global_config(test_inputs, expected, user_filesystem, m # remove the config file from home that came with user_filesystem os.remove(confile) mocker.patch("builtins.input", side_effect=test_inputs["user_inputs"]) - check_and_build_global_config() + actual_bool = check_and_build_global_config() try: with open(confile, "r") as f: actual = json.load(f) + expected_bool = True except FileNotFoundError: + expected_bool = False actual = None assert actual == expected + assert actual_bool == expected_bool def test_check_and_build_global_config_file_exists(user_filesystem, mocker): @@ -121,7 +124,8 @@ def test_check_and_build_global_config_file_exists(user_filesystem, mocker): os.chdir(user_filesystem[1]) confile = user_filesystem[0] / "diffpyconfig.json" expected = {"owner_name": "home_ownername", "owner_email": "home@email.com", "owner_orcid": "home_orcid"} - check_and_build_global_config() + actual_bool = check_and_build_global_config() + assert actual_bool is True with open(confile, "r") as f: actual = json.load(f) assert actual == expected @@ -133,7 +137,8 @@ def test_check_and_build_global_config_skipped(user_filesystem, mocker): confile = user_filesystem[0] / "diffpyconfig.json" # remove the config file from home that came with user_filesystem os.remove(confile) - check_and_build_global_config(skip_config_creation=True) + actual_bool = check_and_build_global_config(skip_config_creation=True) + assert actual_bool is False assert not confile.exists() From 7b99d258409ab4c21e64235a1fc822ecedfd7c95 Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Wed, 25 Dec 2024 08:26:19 -0500 Subject: [PATCH 5/5] update docs with new return value for create_config --- doc/source/examples/tools_example.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/source/examples/tools_example.rst b/doc/source/examples/tools_example.rst index 60746e6f..27de73b5 100644 --- a/doc/source/examples/tools_example.rst +++ b/doc/source/examples/tools_example.rst @@ -95,6 +95,10 @@ it will only run once. However, if you want to bypass this behavior, ``check_and_build_global_config()`` takes an optional boolean ``skip_config_creation`` parameter that could be set to ``True`` at runtime to override the config creation. +``check_and_build_global_config()`` returns ``True`` if the config file exists (whether it created it or not) +and ``False`` if the config file does not exist in the user's home allowing you to develop your own +workflow for handling missing config files after running it with ``skip_config_creation=True``. + I entered the wrong information in my config file so it always loads incorrect information, how do I fix that? --------------------------------------------------------------------------------------------------------------