From eb85d97bc0f1a454299fe79627b8b27161e986fd Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 13 Oct 2025 11:56:23 -0400 Subject: [PATCH 01/28] test for copy_examples --- tests/test_cli.py | 142 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9eb9712..30579c3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,148 @@ import pytest from diffpy.cmi import cli +from diffpy.cmi.packsmanager import PacksManager + + +def safe_get_examples(nonempty_packs, pack, n=None): + """Return up to n examples if available.""" + exs = nonempty_packs.get(pack, []) + if not exs: + return [] + return exs if n is None else exs[:n] + + +def select_examples_subset(examples_dict, copy_type): + """Select subset of examples depending on scenario. + + Handles edge cases where some packs may be empty. + """ + subset = {} + if not examples_dict: + return {} + nonempty_packs = {k: v for k, v in examples_dict.items() if v} + if not nonempty_packs: + return {} + packs = list(nonempty_packs.keys()) + if copy_type == "one_example": + pack = packs[0] + exs = safe_get_examples(nonempty_packs, pack, 1) + if exs: + subset[pack] = exs + elif copy_type == "multiple_examples_same_pack": + pack = packs[0] + exs = safe_get_examples(nonempty_packs, pack, 2) + if exs: + subset[pack] = exs + elif copy_type == "multiple_examples_diff_packs": + for pack in packs[:2]: + exs = safe_get_examples(nonempty_packs, pack, 1) + if exs: + subset[pack] = exs + elif copy_type == "pack_all_examples": + pack = packs[0] + exs = safe_get_examples(nonempty_packs, pack) + if exs: + subset[pack] = exs + elif copy_type == "multi_packs_all_examples": + for pack in packs[:2]: + exs = safe_get_examples(nonempty_packs, pack) + if exs: + subset[pack] = exs + elif copy_type == "combo_packs_and_examples": + first_pack = packs[0] + exs_first = safe_get_examples(nonempty_packs, first_pack, 1) + if exs_first: + subset[first_pack] = exs_first + if len(packs) > 1: + second_pack = packs[1] + exs_second = safe_get_examples(nonempty_packs, second_pack) + if exs_second: + subset[second_pack] = exs_second + elif copy_type == "all_packs_all_examples": + subset = nonempty_packs + else: + raise ValueError(f"Unknown copy_type: {copy_type}") + return subset + + +def verify_copied_files(subset, target_dir, case_dir): + """Verify that all example files were copied correctly.""" + if not subset: + if target_dir: + assert not any(target_dir.iterdir()) + else: + pass + return + dest = target_dir or case_dir + assert dest.exists() + for pack, examples in subset.items(): + for example_name, example_path in examples: + for ex_file in example_path.rglob("*.py"): + rel = ex_file.relative_to(example_path.parent.parent) + dest_file = dest / rel + assert dest_file.exists(), f"{dest_file} missing" + + +# Test copy_examples for various structural cases and copy scenarios. +# In total, 5 structural cases x 14 copy scenarios = 70 tests. +# The copy scenarios are: +# 1a) copy one example to cwd +# 1b) copy one example to a target dir +# 2a) copy multiple examples from same pack to cwd +# 2b) copy multiple examples from same pack to a target dir +# 3a) copy multiple examples from different packs to cwd +# 3b) copy multiple examples from different packs to a target dir +# 4a) copy all examples from a pack to cwd +# 4b) copy all examples from a pack to a target dir +# 5a) copy all examples from multiple packs to cwd +# 5b) copy all examples from multiple packs to target dir +# 6a) copy a combination of packs and examples to cwd +# 6b) copy a combination of packs and examples to target +# 7a) copy all examples from all packs to cwd +# 7b) copy all examples from all packs to a target dir + +copy_scenarios = [ + ("one_example", "cwd"), + ("one_example", "target"), + ("multiple_examples_same_pack", "cwd"), + ("multiple_examples_same_pack", "target"), + ("multiple_examples_diff_packs", "cwd"), + ("multiple_examples_diff_packs", "target"), + ("pack_all_examples", "cwd"), + ("pack_all_examples", "target"), + ("multi_packs_all_examples", "cwd"), + ("multi_packs_all_examples", "target"), + ("combo_packs_and_examples", "cwd"), + ("combo_packs_and_examples", "target"), + ("all_packs_all_examples", "cwd"), + ("all_packs_all_examples", "target"), +] + + +@pytest.mark.parametrize( + "case_name", + ["case1", "case2", "case3", "case4", "case5"], + # ids=lambda x: x.upper(), +) +@pytest.mark.parametrize( + "copy_type, target_type", + copy_scenarios, + # ids=[f"{a}-{b}" for a, b in copy_scenarios], +) +def test_copy_examples( + case_name, copy_type, target_type, example_cases, tmp_path +): + """Test copy_examples for all structural and copy scenarios.""" + case_dir = example_cases / case_name + pm = PacksManager(root_path=case_dir) + examples_dict = pm.available_examples() + target_dir = None if target_type == "cwd" else tmp_path / "target" + if target_dir: + target_dir.mkdir(parents=True, exist_ok=True) + subset = select_examples_subset(examples_dict, copy_type) + cli.copy_examples(subset, target_dir=target_dir) + verify_copied_files(subset, target_dir, case_dir) def test_map_pack_to_examples_structure(): From 918c5598081ed7d004a162f919600621be554a19 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 13 Oct 2025 11:56:59 -0400 Subject: [PATCH 02/28] news --- news/copy-func2.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/copy-func2.rst diff --git a/news/copy-func2.rst b/news/copy-func2.rst new file mode 100644 index 0000000..08bb39a --- /dev/null +++ b/news/copy-func2.rst @@ -0,0 +1,23 @@ +**Added:** + +* Add test for ``copy_examples`` function. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* From 6a26c9e1469c7d104335df6709fdb304b6116f71 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 13 Oct 2025 12:01:21 -0400 Subject: [PATCH 03/28] rm comment --- tests/test_cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 30579c3..f9fc67a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -126,12 +126,10 @@ def verify_copied_files(subset, target_dir, case_dir): @pytest.mark.parametrize( "case_name", ["case1", "case2", "case3", "case4", "case5"], - # ids=lambda x: x.upper(), ) @pytest.mark.parametrize( "copy_type, target_type", copy_scenarios, - # ids=[f"{a}-{b}" for a, b in copy_scenarios], ) def test_copy_examples( case_name, copy_type, target_type, example_cases, tmp_path From 3ab2efbee302a0c21779e863cb40b6fc8f38ecf7 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 13 Oct 2025 14:36:41 -0400 Subject: [PATCH 04/28] rm old tests --- tests/test_cli.py | 62 ----------------------------------------------- 1 file changed, 62 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index f9fc67a..f5cfd2c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,6 +1,3 @@ -import os -from pathlib import Path - import pytest from diffpy.cmi import cli @@ -144,62 +141,3 @@ def test_copy_examples( subset = select_examples_subset(examples_dict, copy_type) cli.copy_examples(subset, target_dir=target_dir) verify_copied_files(subset, target_dir, case_dir) - - -def test_map_pack_to_examples_structure(): - """Test that map_pack_to_examples returns the right shape of - data.""" - actual = cli.map_pack_to_examples() - assert isinstance(actual, dict) - for pack, exdirs in actual.items(): - assert isinstance(pack, str) - assert isinstance(exdirs, list) - for ex in exdirs: - assert isinstance(ex, str) - # Check for known packs - assert "core" in actual.keys() - assert "pdf" in actual.keys() - # Check for known examples - assert ["linefit"] in actual.values() - - -@pytest.mark.parametrize( - "input_valid_str", - [ - "core/linefit", - "pdf/ch03NiModelling", - ], -) -def test_copy_example_success(tmp_path, input_valid_str): - """Given a valid example format (/), test that its copied - to the temp dir.""" - os.chdir(tmp_path) - actual = cli.copy_example(input_valid_str) - expected = tmp_path / Path(input_valid_str).name - assert expected.exists() and expected.is_dir() - assert actual == expected - - -def test_copy_example_fnferror(): - """Test that FileNotFoundError is raised when the example does not - exist.""" - with pytest.raises(FileNotFoundError): - cli.copy_example("pack/example1") - - -@pytest.mark.parametrize( - "input_bad_str", - [ - "", # empty string - "/", # missing pack and example - "corelinefit", # missing slash - "linefit", # missing pack and slash - "core/", # missing example - "/linefit", # missing pack - "core/linefit/extra", # too many slashes - ], -) -def test_copy_example_valueerror(input_bad_str): - """Test that ValueError is raised when the format is invalid.""" - with pytest.raises(ValueError): - cli.copy_example(input_bad_str) From 678d0b99577e61cf5f77f36139d2e6ed25f83cc8 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 10:31:16 -0400 Subject: [PATCH 05/28] add user_input as param for copy_examples --- src/diffpy/cmi/cli.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/diffpy/cmi/cli.py b/src/diffpy/cmi/cli.py index dbd663c..251ae13 100644 --- a/src/diffpy/cmi/cli.py +++ b/src/diffpy/cmi/cli.py @@ -26,7 +26,9 @@ def copy_examples( - examples_dict: Dict[str, List[Tuple[str, Path]]], target_dir: Path = None + examples_dict: Dict[str, List[Tuple[str, Path]]], + user_input: List[str], + target_dir: Path = None, ) -> None: """Copy an example into the the target or current working directory. @@ -34,6 +36,8 @@ def copy_examples( ---------- examples_dict : dict Dictionary mapping pack name -> list of (example, path) tuples. + user_input : list of str + List of user-specified pack(s) or example(s) to copy. target_dir : pathlib.Path, optional Target directory to copy examples into. Defaults to current working directory. From 15fd82aa64f35ffdaa3b34a7a40fe43a3d55b331 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 12:00:05 -0400 Subject: [PATCH 06/28] add user inputs to test --- tests/test_cli.py | 283 +++++++++++++++++++++++++--------------------- 1 file changed, 152 insertions(+), 131 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index f5cfd2c..581203c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,143 +1,164 @@ +from pathlib import Path + import pytest -from diffpy.cmi import cli +from diffpy.cmi.cli import copy_examples from diffpy.cmi.packsmanager import PacksManager - -def safe_get_examples(nonempty_packs, pack, n=None): - """Return up to n examples if available.""" - exs = nonempty_packs.get(pack, []) - if not exs: - return [] - return exs if n is None else exs[:n] - - -def select_examples_subset(examples_dict, copy_type): - """Select subset of examples depending on scenario. - - Handles edge cases where some packs may be empty. - """ - subset = {} - if not examples_dict: - return {} - nonempty_packs = {k: v for k, v in examples_dict.items() if v} - if not nonempty_packs: - return {} - packs = list(nonempty_packs.keys()) - if copy_type == "one_example": - pack = packs[0] - exs = safe_get_examples(nonempty_packs, pack, 1) - if exs: - subset[pack] = exs - elif copy_type == "multiple_examples_same_pack": - pack = packs[0] - exs = safe_get_examples(nonempty_packs, pack, 2) - if exs: - subset[pack] = exs - elif copy_type == "multiple_examples_diff_packs": - for pack in packs[:2]: - exs = safe_get_examples(nonempty_packs, pack, 1) - if exs: - subset[pack] = exs - elif copy_type == "pack_all_examples": - pack = packs[0] - exs = safe_get_examples(nonempty_packs, pack) - if exs: - subset[pack] = exs - elif copy_type == "multi_packs_all_examples": - for pack in packs[:2]: - exs = safe_get_examples(nonempty_packs, pack) - if exs: - subset[pack] = exs - elif copy_type == "combo_packs_and_examples": - first_pack = packs[0] - exs_first = safe_get_examples(nonempty_packs, first_pack, 1) - if exs_first: - subset[first_pack] = exs_first - if len(packs) > 1: - second_pack = packs[1] - exs_second = safe_get_examples(nonempty_packs, second_pack) - if exs_second: - subset[second_pack] = exs_second - elif copy_type == "all_packs_all_examples": - subset = nonempty_packs - else: - raise ValueError(f"Unknown copy_type: {copy_type}") - return subset - - -def verify_copied_files(subset, target_dir, case_dir): - """Verify that all example files were copied correctly.""" - if not subset: - if target_dir: - assert not any(target_dir.iterdir()) - else: - pass - return - dest = target_dir or case_dir - assert dest.exists() - for pack, examples in subset.items(): - for example_name, example_path in examples: - for ex_file in example_path.rglob("*.py"): - rel = ex_file.relative_to(example_path.parent.parent) - dest_file = dest / rel - assert dest_file.exists(), f"{dest_file} missing" - - # Test copy_examples for various structural cases and copy scenarios. -# In total, 5 structural cases x 14 copy scenarios = 70 tests. +# In total, 5 structural cases x 6 copy scenarios + 2 path scenarios = 32 tests # The copy scenarios are: -# 1a) copy one example to cwd -# 1b) copy one example to a target dir -# 2a) copy multiple examples from same pack to cwd -# 2b) copy multiple examples from same pack to a target dir -# 3a) copy multiple examples from different packs to cwd -# 3b) copy multiple examples from different packs to a target dir -# 4a) copy all examples from a pack to cwd -# 4b) copy all examples from a pack to a target dir -# 5a) copy all examples from multiple packs to cwd -# 5b) copy all examples from multiple packs to target dir -# 6a) copy a combination of packs and examples to cwd -# 6b) copy a combination of packs and examples to target -# 7a) copy all examples from all packs to cwd -# 7b) copy all examples from all packs to a target dir +# a) copy to cwd +# b) copy to target dir -copy_scenarios = [ - ("one_example", "cwd"), - ("one_example", "target"), - ("multiple_examples_same_pack", "cwd"), - ("multiple_examples_same_pack", "target"), - ("multiple_examples_diff_packs", "cwd"), - ("multiple_examples_diff_packs", "target"), - ("pack_all_examples", "cwd"), - ("pack_all_examples", "target"), - ("multi_packs_all_examples", "cwd"), - ("multi_packs_all_examples", "target"), - ("combo_packs_and_examples", "cwd"), - ("combo_packs_and_examples", "target"), - ("all_packs_all_examples", "cwd"), - ("all_packs_all_examples", "target"), +# 1) copy one example +# 2) copy list of examples from same pack +# 3) copy list of examples from different packs +# 4) copy all examples from a pack +# 5) copy all examples from list of packs +# 6) copy all examples from all packs + +# Case params: List of tuples +# input: str - name of the test case directory +# user_inputs: List[List[str]] - Possible user inputs for the given case +# expected: List[List[Path]] - The expected paths that should be copied for +# each user input +case_params = [ + ( + "case1", # empty pack + [ + ["empty_pack"], # 4) all examples from a pack (but pack is empty) + ["all"], # 6) all examples from all packs (but pack is empty) + ], + [ + [], + [], + ], + ), + ( + "case2", # one pack with multiple examples + [ + ["ex1"], # 1) single example + ["ex1", "ex2"], # 2) multiple examples from same pack + ["full_pack"], # 4) all examples from a pack + ["all"], # 6) all examples from all packs + ], + [ + [Path("full_pack/ex1/solution/diffpy-cmi/script1.py")], + [ + Path("full_pack/ex1/solution/diffpy-cmi/script1.py"), + Path("full_pack/ex2/random/path/script1.py"), + Path("full_pack/ex2/random/path/script2.py"), + ], + [ + Path("full_pack/ex1/solution/diffpy-cmi/script1.py"), + Path("full_pack/ex2/random/path/script1.py"), + Path("full_pack/ex2/random/path/script2.py"), + ], + [ + Path("full_pack/ex1/solution/diffpy-cmi/script1.py"), + Path("full_pack/ex2/random/path/script1.py"), + Path("full_pack/ex2/random/path/script2.py"), + ], + ], + ), + ( + "case3", # multiple packs with multiple examples + [ + ["ex1"], # 1) single example from packA + ["ex1", "ex2"], # 2) list of examples from same pack + ["ex2", "ex3"], # 3) list of examples from different packs + ["packA"], # 4) all examples from a pack + ["packA", "packB"], # 5) all examples from multiple packs + ["all"], # 6) all examples from all packs + ], + [ + [Path("packA/ex1/script1.py")], + [ + Path("packA/ex1/script1.py"), + Path("packA/ex2/solutions/script2.py"), + ], + [ + Path("packA/ex2/solutions/script2.py"), + Path("packB/ex3/more/random/path/script3.py"), + Path("packB/ex3/more/random/path/script4.py"), + ], + [ + Path("packA/ex1/script1.py"), + Path("packA/ex2/solutions/script2.py"), + ], + [ + Path("packA/ex1/script1.py"), + Path("packA/ex2/solutions/script2.py"), + Path("packB/ex3/more/random/path/script3.py"), + Path("packB/ex3/more/random/path/script4.py"), + ], + [ + Path("packA/ex1/script1.py"), + Path("packA/ex2/solutions/script2.py"), + Path("packB/ex3/more/random/path/script3.py"), + Path("packB/ex3/more/random/path/script4.py"), + ], + ], + ), + ( + "case4", # no packs (empty examples directory) + [ + ["all"], # 6) all examples from all packs (but examples exist) + ], + [ + [], + ], + ), + ( + "case5", # multiple packs containing examples with the same name + [ + ["ex1"], # 1) single example (ambiguous, should get both) + [ + "ex1", + "ex1", + ], # 3) list of ex from diff packs (ambiguous, should get both) + ["packA"], # 4) all examples from a pack + ["packA", "packB"], # 5) all examples from a list of packs + ["all"], # 6) all examples from all packs + ], + [ + [ + Path("packA/ex1/path1/script1.py"), + Path("packB/ex1/path2/script2.py"), + ], + [ + Path("packA/ex1/path1/script1.py"), + Path("packB/ex1/path2/script2.py"), + ], + [Path("packA/ex1/path1/script1.py")], + [ + Path("packA/ex1/path1/script1.py"), + Path("packB/ex1/path2/script2.py"), + ], + [ + Path("packA/ex1/path1/script1.py"), + Path("packA/ex1/path2/script2.py"), + ], + ], + ), ] -@pytest.mark.parametrize( - "case_name", - ["case1", "case2", "case3", "case4", "case5"], -) -@pytest.mark.parametrize( - "copy_type, target_type", - copy_scenarios, -) -def test_copy_examples( - case_name, copy_type, target_type, example_cases, tmp_path -): - """Test copy_examples for all structural and copy scenarios.""" - case_dir = example_cases / case_name +@pytest.mark.parametrize("target", ["cwd", "target"]) +@pytest.mark.parametrize("input,user_inputs,expected", case_params) +def test_copy_examples(input, expected, user_inputs, target, example_cases): + case_dir = example_cases / input pm = PacksManager(root_path=case_dir) examples_dict = pm.available_examples() - target_dir = None if target_type == "cwd" else tmp_path / "target" - if target_dir: - target_dir.mkdir(parents=True, exist_ok=True) - subset = select_examples_subset(examples_dict, copy_type) - cli.copy_examples(subset, target_dir=target_dir) - verify_copied_files(subset, target_dir, case_dir) + if target == "target": + target_dir = case_dir / "target_dir" + else: + target_dir = case_dir + target_dir.mkdir(parents=True, exist_ok=True) + for command in user_inputs: + copy_examples(examples_dict, user_input=command, target_dir=target_dir) + for exp_paths in expected: + for path in exp_paths: + dest_file = target_dir / path + assert dest_file.exists(), f"{dest_file} missing" From d57e7fa701173218d6ea8ffe2faf5e00552e354c Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 13:42:59 -0400 Subject: [PATCH 07/28] add cwd and user_target at same level as cases in tmp dir --- tests/conftest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5a4edca..321a535 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -30,7 +30,10 @@ def example_cases(tmp_path_factory): Returns the path to that copy. """ root_temp_dir = tmp_path_factory.mktemp("temp") - + cwd = root_temp_dir / "cwd" + cwd.mkdir(parents=True, exist_ok=True) + user_target = root_temp_dir / "user_target" + user_target.mkdir(parents=True, exist_ok=True) # case 1: pack with no examples case1ex_dir = root_temp_dir / "case1" / "docs" / "examples" case1 = case1ex_dir / "empty_pack" # empty_pack From 79fa6791af9620267a8dd050c7605dd34ce4f592 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 13:43:31 -0400 Subject: [PATCH 08/28] modify test for new tmp dir structure --- tests/test_cli.py | 49 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 581203c..f47ba2b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,3 +1,4 @@ +import os from pathlib import Path import pytest @@ -25,7 +26,6 @@ # each user input case_params = [ ( - "case1", # empty pack [ ["empty_pack"], # 4) all examples from a pack (but pack is empty) ["all"], # 6) all examples from all packs (but pack is empty) @@ -36,7 +36,6 @@ ], ), ( - "case2", # one pack with multiple examples [ ["ex1"], # 1) single example ["ex1", "ex2"], # 2) multiple examples from same pack @@ -63,7 +62,6 @@ ], ), ( - "case3", # multiple packs with multiple examples [ ["ex1"], # 1) single example from packA ["ex1", "ex2"], # 2) list of examples from same pack @@ -102,7 +100,6 @@ ], ), ( - "case4", # no packs (empty examples directory) [ ["all"], # 6) all examples from all packs (but examples exist) ], @@ -111,7 +108,6 @@ ], ), ( - "case5", # multiple packs containing examples with the same name [ ["ex1"], # 1) single example (ambiguous, should get both) [ @@ -145,20 +141,43 @@ ] -@pytest.mark.parametrize("target", ["cwd", "target"]) -@pytest.mark.parametrize("input,user_inputs,expected", case_params) -def test_copy_examples(input, expected, user_inputs, target, example_cases): - case_dir = example_cases / input +@pytest.mark.parametrize( + "case,target", + [ + ("case1", None), + ("case1", "user_target"), + ("case2", None), + ("case2", "user_target"), + ("case3", None), + ("case3", "user_target"), + ("case4", None), + ("case4", "user_target"), + ("case5", None), + ("case5", "user_target"), + ], +) +@pytest.mark.parametrize("user_inputs,expected", case_params) +def test_copy_examples(case, user_inputs, expected, target, example_cases): + cwd = example_cases / "cwd" + os.chdir(cwd) + case_dir = example_cases / case pm = PacksManager(root_path=case_dir) examples_dict = pm.available_examples() - if target == "target": - target_dir = case_dir / "target_dir" + if target is None: + target_dir = cwd else: - target_dir = case_dir - target_dir.mkdir(parents=True, exist_ok=True) + target_dir = case_dir / "user_target" for command in user_inputs: - copy_examples(examples_dict, user_input=command, target_dir=target_dir) + copy_examples( + examples_dict, + user_input=command, + target_dir=None if target is None else target_dir, + ) + if expected: for exp_paths in expected: for path in exp_paths: dest_file = target_dir / path - assert dest_file.exists(), f"{dest_file} missing" + assert dest_file.exists() + else: + empty_dir = list(target_dir.rglob("*")) + assert not empty_dir, f"Expected nothing, but found: {empty_dir}" From a1fcf3b87cd9229b382d107d3f7736a6e41ce94e Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 15:27:52 -0400 Subject: [PATCH 09/28] fix testing matrix --- tests/test_cli.py | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index f47ba2b..6c2a4d5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -26,6 +26,7 @@ # each user input case_params = [ ( + "case1", # case with empty pack [ ["empty_pack"], # 4) all examples from a pack (but pack is empty) ["all"], # 6) all examples from all packs (but pack is empty) @@ -36,6 +37,7 @@ ], ), ( + "case2", # case with one pack with multiple examples [ ["ex1"], # 1) single example ["ex1", "ex2"], # 2) multiple examples from same pack @@ -62,6 +64,7 @@ ], ), ( + "case3", # case with multiple packs with multiple examples [ ["ex1"], # 1) single example from packA ["ex1", "ex2"], # 2) list of examples from same pack @@ -100,6 +103,7 @@ ], ), ( + "case4", # case with no packs (empty examples directory) [ ["all"], # 6) all examples from all packs (but examples exist) ], @@ -108,6 +112,7 @@ ], ), ( + "case5", # case with multiple packs with same example names [ ["ex1"], # 1) single example (ambiguous, should get both) [ @@ -141,22 +146,8 @@ ] -@pytest.mark.parametrize( - "case,target", - [ - ("case1", None), - ("case1", "user_target"), - ("case2", None), - ("case2", "user_target"), - ("case3", None), - ("case3", "user_target"), - ("case4", None), - ("case4", "user_target"), - ("case5", None), - ("case5", "user_target"), - ], -) -@pytest.mark.parametrize("user_inputs,expected", case_params) +@pytest.mark.parametrize("target", [None, "user_target"]) +@pytest.mark.parametrize("case,user_inputs,expected", case_params) def test_copy_examples(case, user_inputs, expected, target, example_cases): cwd = example_cases / "cwd" os.chdir(cwd) From fc1d0b0d72d70b9c818e7274bc47ad76384b5d1f Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 16:52:47 -0400 Subject: [PATCH 10/28] add test for bad copy cases --- tests/test_cli.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 6c2a4d5..d9718fb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -172,3 +172,24 @@ def test_copy_examples(case, user_inputs, expected, target, example_cases): else: empty_dir = list(target_dir.rglob("*")) assert not empty_dir, f"Expected nothing, but found: {empty_dir}" + + +# Test bad inputs to copy_examples +# These include: +# 1) input not found (example or pack) +# 2) mixed good and bad inputs +@pytest.mark.parametrize("case", ["case1", "case2", "case3", "case4", "case5"]) +@pytest.mark.parametrize( + "bad_inputs, expected", + [ + (["bad_example"], ValueError), # input not found (example or pack) + (["ex1", "bad_example"], ValueError), # mixed good and bad inputs + ], +) +def test_copy_examples_bad(bad_inputs, expected, case, example_cases): + """Test copy_examples with bad inputs.""" + case_dir = example_cases / case + pm = PacksManager(root_path=case_dir) + examples_dict = pm.available_examples() + with pytest.raises(expected): + copy_examples(examples_dict, user_input=bad_inputs) From a50c4428cbb1f260def2ac61cc4bf420a994c897 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 16:53:28 -0400 Subject: [PATCH 11/28] rm docstring --- tests/test_cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index d9718fb..2070a16 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -187,7 +187,6 @@ def test_copy_examples(case, user_inputs, expected, target, example_cases): ], ) def test_copy_examples_bad(bad_inputs, expected, case, example_cases): - """Test copy_examples with bad inputs.""" case_dir = example_cases / case pm = PacksManager(root_path=case_dir) examples_dict = pm.available_examples() From a04de5a558bdbaa098bc42815b1e0462ea203fbc Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 17:07:29 -0400 Subject: [PATCH 12/28] Add FileExistError test case --- tests/test_cli.py | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 2070a16..b3fca2b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -174,21 +174,43 @@ def test_copy_examples(case, user_inputs, expected, target, example_cases): assert not empty_dir, f"Expected nothing, but found: {empty_dir}" -# Test bad inputs to copy_examples +# Test bad inputs to copy_examples on case3 # These include: # 1) input not found (example or pack) # 2) mixed good and bad inputs -@pytest.mark.parametrize("case", ["case1", "case2", "case3", "case4", "case5"]) +# 3) Path to directory already exists @pytest.mark.parametrize( - "bad_inputs, expected", + "bad_inputs, expected, path", [ - (["bad_example"], ValueError), # input not found (example or pack) - (["ex1", "bad_example"], ValueError), # mixed good and bad inputs + ( + ["bad_example"], + ValueError, + None, + ), # input not found (example or pack) + ( + ["ex1", "bad_example"], + ValueError, + None, + ), # mixed good ex and bad inputs + ( + ["packA", "bad_example"], + ValueError, + None, + ), # mixed good pack and bad inputs + ( + ["ex1"], + FileExistsError, + Path("docs/examples/"), + ), # path to dir already exists ], ) -def test_copy_examples_bad(bad_inputs, expected, case, example_cases): - case_dir = example_cases / case +def test_copy_examples_bad(bad_inputs, expected, path, example_cases): + case_dir = example_cases / "case3" pm = PacksManager(root_path=case_dir) examples_dict = pm.available_examples() with pytest.raises(expected): - copy_examples(examples_dict, user_input=bad_inputs) + copy_examples( + examples_dict, + user_input=bad_inputs, + target_dir=case_dir / path if path else None, + ) From aa453b7b01d3e839fe2d16e6a38369b6006a6e73 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 14 Oct 2025 17:08:43 -0400 Subject: [PATCH 13/28] reverse logic for target_dir --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index b3fca2b..941c331 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -212,5 +212,5 @@ def test_copy_examples_bad(bad_inputs, expected, path, example_cases): copy_examples( examples_dict, user_input=bad_inputs, - target_dir=case_dir / path if path else None, + target_dir=case_dir / path if path is not None else None, ) From e75a339aa25d4d8d449302625d28fb238d6d280e Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 15 Oct 2025 14:44:21 -0400 Subject: [PATCH 14/28] add additional dirs to tmp dir --- tests/conftest.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 321a535..4621573 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -89,6 +89,15 @@ def example_cases(tmp_path_factory): case5b = case5ex_dir / "packB" / "ex1" / "path2" # packB, ex1 case5b.mkdir(parents=True, exist_ok=True) (case5b / "script2.py").touch() + case5c = case5ex_dir / "packA" / "ex2" # packB, ex2 + case5c.mkdir(parents=True, exist_ok=True) + (case5c / "script3.py").touch() + case5d = case5ex_dir / "packB" / "ex3" + case5d.mkdir(parents=True, exist_ok=True) + (case5d / "script4.py").touch() + case5e = case5ex_dir / "packB" / "ex4" + case5e.mkdir(parents=True, exist_ok=True) + (case5e / "script5.py").touch() case5req_dir = root_temp_dir / "case5" / "requirements" / "packs" case5req_dir.mkdir(parents=True, exist_ok=True) From f750453b936565ef58935c865b59738668a68d41 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 15 Oct 2025 14:48:15 -0400 Subject: [PATCH 15/28] move copy_examples inside PacksManager --- src/diffpy/cmi/packsmanager.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/diffpy/cmi/packsmanager.py b/src/diffpy/cmi/packsmanager.py index b33ae83..e45665a 100644 --- a/src/diffpy/cmi/packsmanager.py +++ b/src/diffpy/cmi/packsmanager.py @@ -113,6 +113,24 @@ def available_examples(self) -> dict[str, List[tuple[str, Path]]]: ) return examples_dict + def copy_examples( + self, + user_input: Union[str, List[str]], + target_dir: Path = None, + ) -> List[Path]: + """Copy examples or packs into the target or current working + directory. + + Parameters + ---------- + user_input : str or list of str + User-specified pack(s), example(s), or "all" to copy all. + target_dir : pathlib.Path, optional + Target directory to copy examples into. Defaults to current + working directory. + """ + return + def _resolve_pack_file(self, identifier: Union[str, Path]) -> Path: """Resolve a pack identifier to an absolute .txt path. From 49e745e93fc4cc9f98fde1b6713a1fe3c316670d Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 15 Oct 2025 14:48:51 -0400 Subject: [PATCH 16/28] move copy_examples tests inside pm test --- tests/test_packsmanager.py | 192 +++++++++++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 986b060..89e590c 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -1,3 +1,6 @@ +import os +from pathlib import Path + import pytest from diffpy.cmi.packsmanager import PacksManager @@ -63,9 +66,12 @@ def paths_and_names_match(expected, actual, root): { "packA": [ ("ex1", "case5/docs/examples/packA/ex1"), + ("ex2", "case5/docs/examples/packA/ex2"), ], "packB": [ ("ex1", "case5/docs/examples/packB/ex1"), + ("ex3", "case5/docs/examples/packB/ex3"), + ("ex4", "case5/docs/examples/packB/ex4"), ], }, ), @@ -92,3 +98,189 @@ def test_tmp_file_structure(input, expected, example_cases): assert path.is_file() else: assert path.is_dir() + + +copy_params = [ + # Test various use cases to copy_examples on case5 + # 1) copy one example (ambiguous) + # 2) copy list of examples from same pack (ambiguous) + # 3) copy list of examples from different pack (ambiguous) + # 4) copy one example (unambiguous) + # 5) copy list of examples from same pack (unambiguous) + # 6) copy list of examples from different pack (unambiguous) + # 7) copy all examples from a pack + # 8) copy all examples from list of packs + # 9) copy all examples from all packs + ( # 1) copy one example, (ambiguous) + "ex1", + [ + Path("packA/ex1/path1/script1.py"), + Path("packB/ex1/path2/script2.py"), + ], + ), + ( # 2) copy list of examples from same pack (ambiguous) + ["ex1", "ex2"], + [ + Path("packA/ex1/path1/script1.py"), + Path("packB/ex1/path2/script2.py"), + Path("packA/ex2/script3.py"), + ], + ), + ( # 3) copy list of examples from different packs (ambiguous) + ["ex1", "ex1"], + [ + Path("packA/ex1/path1/script1.py"), + Path("packB/ex1/path2/script2.py"), + ], + ), + ( # 4) copy one example (unambiguous) + "ex2", + [ + Path("packA/ex2/script3.py"), + ], + ), + ( # 5) copy list of examples from same pack (unambiguous) + ["ex3", "ex4"], + [ + Path("packB/ex3/script4.py"), + Path("packB/ex4/script5.py"), + ], + ), + ( # 6) copy list of examples from different packs (unambiguous) + ["ex2", "ex3"], + [ + Path("packA/ex2/script3.py"), + Path("packB/ex3/script4.py"), + ], + ), + ( # 7) copy all examples from a pack + "packA", + [ + Path("packA/ex1/path1/script1.py"), + Path("packA/ex2/script3.py"), + ], + ), + ( # 8) copy all examples from list of packs + ["packA", "packB"], + [ + Path("packA/ex1/path1/script1.py"), + Path("packA/ex2/script3.py"), + Path("packB/ex1/path2/script2.py"), + Path("packB/ex3/script4.py"), + Path("packB/ex4/script5.py"), + ], + ), + ( # 9) copy all examples from all packs + "all", + [ + Path("packA/ex1/path1/script1.py"), + Path("packA/ex2/script3.py"), + Path("packB/ex1/path2/script2.py"), + Path("packB/ex3/script4.py"), + Path("packB/ex4/script5.py"), + ], + ), +] + + +# input: list of str - cli input(s) to copy_examples +# expected_paths: list of Path - expected relative paths to copied examples +@pytest.mark.parametrize("input,expected_paths", copy_params) +def test_copy_examples(input, expected_paths, example_cases): + examples_dir = example_cases / "case5" + pm = PacksManager(root_path=examples_dir) + target_dir = example_cases / "user_target" + actual = pm.copy_examples(user_input=input, target_dir=target_dir) + expected = [] + for path in expected_paths: + root_path = target_dir / path + expected.append(root_path) + assert actual == expected + + +# Test default and targeted copy_example location on case5 +# input: str or None - path arg to copy_examples +# expected: Path - expected relative path to copied example +@pytest.mark.parametrize( + "input,expected_path", + [ + (None, Path("cwd/packA/ex1/path1/script1.py")), + ("user_target", Path("user_target/packA/ex1/path1/script1.py")), + ], +) +def test_copy_example_location(input, expected_path, example_cases): + examples_dir = example_cases / "case5" + os.chdir(example_cases / "cwd") + pm = PacksManager(root_path=examples_dir) + paths = pm.copy_examples(user_input="packA", target_dir=input) + actual = paths[0] + expected = example_cases / expected_path + assert actual == expected + + +# Test bad inputs to copy_examples on case3 +# These include: +# 1) input not found (example or pack) +# 2) mixed good and bad inputs +# 3) Path to directory already exists +@pytest.mark.parametrize( + "bad_inputs,expected,path", + [ + ( + "bad_example", + ValueError, + None, + ), # input not found (example or pack) + ( + ["ex1", "bad_example"], + ValueError, + None, + ), # mixed good ex and bad inputs + ( + ["packA", "bad_example"], + ValueError, + None, + ), # mixed good pack and bad inputs + ( + "ex1", + FileExistsError, + Path("docs/examples/"), + ), # path to dir already exists + ], +) +def test_copy_examples_bad(bad_inputs, expected, path, example_cases): + examples_dir = example_cases / "case3" + pm = PacksManager(root_path=examples_dir) + with pytest.raises(expected): + pm.copy_examples( + user_input=bad_inputs, + target_dir=examples_dir / path if path is not None else None, + ) + + +# Test bad target_dir to copy_examples on case3 +# 1) target doesn't exist +# 2) target is a file +# 3) target is nested in a file +@pytest.mark.parametrize( + "bad_inputs,expected", + [ + (Path("nonexistent/path/target"), FileNotFoundError), # doesn't exist + ( + Path("docs/examples/packA/ex1/script4.py"), + ValueError, + ), # target is a file + ( + Path("docs/examples/packA/ex1/script4.py/nested"), + ValueError, + ), # nested in file + ], +) +def test_copy_examples_bad_target(bad_inputs, expected, example_cases): + examples_dir = example_cases / "case3" + pm = PacksManager(root_path=examples_dir) + with pytest.raises(NotADirectoryError): + pm.copy_examples( + user_input=["packA"], + target_dir=examples_dir / "docs/examples/packA/ex1/script4.py", + ) From 14fac597b1b189532a77cdb1e4431addacc1e183 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 15 Oct 2025 14:49:06 -0400 Subject: [PATCH 17/28] rm tests from test_cli --- tests/test_cli.py | 216 ---------------------------------------------- 1 file changed, 216 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 941c331..e69de29 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,216 +0,0 @@ -import os -from pathlib import Path - -import pytest - -from diffpy.cmi.cli import copy_examples -from diffpy.cmi.packsmanager import PacksManager - -# Test copy_examples for various structural cases and copy scenarios. -# In total, 5 structural cases x 6 copy scenarios + 2 path scenarios = 32 tests -# The copy scenarios are: -# a) copy to cwd -# b) copy to target dir - -# 1) copy one example -# 2) copy list of examples from same pack -# 3) copy list of examples from different packs -# 4) copy all examples from a pack -# 5) copy all examples from list of packs -# 6) copy all examples from all packs - -# Case params: List of tuples -# input: str - name of the test case directory -# user_inputs: List[List[str]] - Possible user inputs for the given case -# expected: List[List[Path]] - The expected paths that should be copied for -# each user input -case_params = [ - ( - "case1", # case with empty pack - [ - ["empty_pack"], # 4) all examples from a pack (but pack is empty) - ["all"], # 6) all examples from all packs (but pack is empty) - ], - [ - [], - [], - ], - ), - ( - "case2", # case with one pack with multiple examples - [ - ["ex1"], # 1) single example - ["ex1", "ex2"], # 2) multiple examples from same pack - ["full_pack"], # 4) all examples from a pack - ["all"], # 6) all examples from all packs - ], - [ - [Path("full_pack/ex1/solution/diffpy-cmi/script1.py")], - [ - Path("full_pack/ex1/solution/diffpy-cmi/script1.py"), - Path("full_pack/ex2/random/path/script1.py"), - Path("full_pack/ex2/random/path/script2.py"), - ], - [ - Path("full_pack/ex1/solution/diffpy-cmi/script1.py"), - Path("full_pack/ex2/random/path/script1.py"), - Path("full_pack/ex2/random/path/script2.py"), - ], - [ - Path("full_pack/ex1/solution/diffpy-cmi/script1.py"), - Path("full_pack/ex2/random/path/script1.py"), - Path("full_pack/ex2/random/path/script2.py"), - ], - ], - ), - ( - "case3", # case with multiple packs with multiple examples - [ - ["ex1"], # 1) single example from packA - ["ex1", "ex2"], # 2) list of examples from same pack - ["ex2", "ex3"], # 3) list of examples from different packs - ["packA"], # 4) all examples from a pack - ["packA", "packB"], # 5) all examples from multiple packs - ["all"], # 6) all examples from all packs - ], - [ - [Path("packA/ex1/script1.py")], - [ - Path("packA/ex1/script1.py"), - Path("packA/ex2/solutions/script2.py"), - ], - [ - Path("packA/ex2/solutions/script2.py"), - Path("packB/ex3/more/random/path/script3.py"), - Path("packB/ex3/more/random/path/script4.py"), - ], - [ - Path("packA/ex1/script1.py"), - Path("packA/ex2/solutions/script2.py"), - ], - [ - Path("packA/ex1/script1.py"), - Path("packA/ex2/solutions/script2.py"), - Path("packB/ex3/more/random/path/script3.py"), - Path("packB/ex3/more/random/path/script4.py"), - ], - [ - Path("packA/ex1/script1.py"), - Path("packA/ex2/solutions/script2.py"), - Path("packB/ex3/more/random/path/script3.py"), - Path("packB/ex3/more/random/path/script4.py"), - ], - ], - ), - ( - "case4", # case with no packs (empty examples directory) - [ - ["all"], # 6) all examples from all packs (but examples exist) - ], - [ - [], - ], - ), - ( - "case5", # case with multiple packs with same example names - [ - ["ex1"], # 1) single example (ambiguous, should get both) - [ - "ex1", - "ex1", - ], # 3) list of ex from diff packs (ambiguous, should get both) - ["packA"], # 4) all examples from a pack - ["packA", "packB"], # 5) all examples from a list of packs - ["all"], # 6) all examples from all packs - ], - [ - [ - Path("packA/ex1/path1/script1.py"), - Path("packB/ex1/path2/script2.py"), - ], - [ - Path("packA/ex1/path1/script1.py"), - Path("packB/ex1/path2/script2.py"), - ], - [Path("packA/ex1/path1/script1.py")], - [ - Path("packA/ex1/path1/script1.py"), - Path("packB/ex1/path2/script2.py"), - ], - [ - Path("packA/ex1/path1/script1.py"), - Path("packA/ex1/path2/script2.py"), - ], - ], - ), -] - - -@pytest.mark.parametrize("target", [None, "user_target"]) -@pytest.mark.parametrize("case,user_inputs,expected", case_params) -def test_copy_examples(case, user_inputs, expected, target, example_cases): - cwd = example_cases / "cwd" - os.chdir(cwd) - case_dir = example_cases / case - pm = PacksManager(root_path=case_dir) - examples_dict = pm.available_examples() - if target is None: - target_dir = cwd - else: - target_dir = case_dir / "user_target" - for command in user_inputs: - copy_examples( - examples_dict, - user_input=command, - target_dir=None if target is None else target_dir, - ) - if expected: - for exp_paths in expected: - for path in exp_paths: - dest_file = target_dir / path - assert dest_file.exists() - else: - empty_dir = list(target_dir.rglob("*")) - assert not empty_dir, f"Expected nothing, but found: {empty_dir}" - - -# Test bad inputs to copy_examples on case3 -# These include: -# 1) input not found (example or pack) -# 2) mixed good and bad inputs -# 3) Path to directory already exists -@pytest.mark.parametrize( - "bad_inputs, expected, path", - [ - ( - ["bad_example"], - ValueError, - None, - ), # input not found (example or pack) - ( - ["ex1", "bad_example"], - ValueError, - None, - ), # mixed good ex and bad inputs - ( - ["packA", "bad_example"], - ValueError, - None, - ), # mixed good pack and bad inputs - ( - ["ex1"], - FileExistsError, - Path("docs/examples/"), - ), # path to dir already exists - ], -) -def test_copy_examples_bad(bad_inputs, expected, path, example_cases): - case_dir = example_cases / "case3" - pm = PacksManager(root_path=case_dir) - examples_dict = pm.available_examples() - with pytest.raises(expected): - copy_examples( - examples_dict, - user_input=bad_inputs, - target_dir=case_dir / path if path is not None else None, - ) From 909da70f5d1a2f5b63190f503a298730d1c92bd9 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 15 Oct 2025 14:56:05 -0400 Subject: [PATCH 18/28] add bad_target test --- tests/test_packsmanager.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 89e590c..6933e44 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -208,7 +208,7 @@ def test_copy_examples(input, expected_paths, example_cases): ("user_target", Path("user_target/packA/ex1/path1/script1.py")), ], ) -def test_copy_example_location(input, expected_path, example_cases): +def test_copy_examples_location(input, expected_path, example_cases): examples_dir = example_cases / "case5" os.chdir(example_cases / "cwd") pm = PacksManager(root_path=examples_dir) @@ -268,19 +268,19 @@ def test_copy_examples_bad(bad_inputs, expected, path, example_cases): (Path("nonexistent/path/target"), FileNotFoundError), # doesn't exist ( Path("docs/examples/packA/ex1/script4.py"), - ValueError, + NotADirectoryError, ), # target is a file ( Path("docs/examples/packA/ex1/script4.py/nested"), - ValueError, + NotADirectoryError, ), # nested in file ], ) def test_copy_examples_bad_target(bad_inputs, expected, example_cases): examples_dir = example_cases / "case3" pm = PacksManager(root_path=examples_dir) - with pytest.raises(NotADirectoryError): + with pytest.raises(expected): pm.copy_examples( - user_input=["packA"], - target_dir=examples_dir / "docs/examples/packA/ex1/script4.py", + user_input="packA", + target_dir=bad_inputs, ) From e66eb5f4d1c689ccecdf7095d70c0a4f06e12a9e Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Thu, 16 Oct 2025 10:47:14 -0400 Subject: [PATCH 19/28] rm duplicate copy_examples() in cli.py --- src/diffpy/cmi/cli.py | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/diffpy/cmi/cli.py b/src/diffpy/cmi/cli.py index 251ae13..f25cb78 100644 --- a/src/diffpy/cmi/cli.py +++ b/src/diffpy/cmi/cli.py @@ -16,7 +16,7 @@ import argparse from pathlib import Path from shutil import copytree -from typing import Dict, List, Optional, Tuple +from typing import List, Optional, Tuple from diffpy.cmi import __version__, get_package_dir from diffpy.cmi.conda import env_info @@ -25,26 +25,6 @@ from diffpy.cmi.profilesmanager import ProfilesManager -def copy_examples( - examples_dict: Dict[str, List[Tuple[str, Path]]], - user_input: List[str], - target_dir: Path = None, -) -> None: - """Copy an example into the the target or current working directory. - - Parameters - ---------- - examples_dict : dict - Dictionary mapping pack name -> list of (example, path) tuples. - user_input : list of str - List of user-specified pack(s) or example(s) to copy. - target_dir : pathlib.Path, optional - Target directory to copy examples into. Defaults to current - working directory. - """ - return - - # Examples def _get_examples_dir() -> Path: """Return the absolute path to the installed examples directory. From 42553c117fd09577d1e269f07e5481b16d6654d0 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Thu, 16 Oct 2025 10:47:43 -0400 Subject: [PATCH 20/28] update docstring and param name --- src/diffpy/cmi/packsmanager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/diffpy/cmi/packsmanager.py b/src/diffpy/cmi/packsmanager.py index e45665a..c7831d0 100644 --- a/src/diffpy/cmi/packsmanager.py +++ b/src/diffpy/cmi/packsmanager.py @@ -115,7 +115,7 @@ def available_examples(self) -> dict[str, List[tuple[str, Path]]]: def copy_examples( self, - user_input: Union[str, List[str]], + examples_to_copy: List[str], target_dir: Path = None, ) -> List[Path]: """Copy examples or packs into the target or current working @@ -123,7 +123,7 @@ def copy_examples( Parameters ---------- - user_input : str or list of str + examples_to_copy : list of str User-specified pack(s), example(s), or "all" to copy all. target_dir : pathlib.Path, optional Target directory to copy examples into. Defaults to current From ef792d2cf9674f1038b78e50862a0bbfc1f3e7f6 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Thu, 16 Oct 2025 10:49:03 -0400 Subject: [PATCH 21/28] wrap str as list, add 4) to bad input test --- tests/test_packsmanager.py | 46 +++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 6933e44..a9d7562 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -112,7 +112,7 @@ def test_tmp_file_structure(input, expected, example_cases): # 8) copy all examples from list of packs # 9) copy all examples from all packs ( # 1) copy one example, (ambiguous) - "ex1", + ["ex1"], [ Path("packA/ex1/path1/script1.py"), Path("packB/ex1/path2/script2.py"), @@ -134,7 +134,7 @@ def test_tmp_file_structure(input, expected, example_cases): ], ), ( # 4) copy one example (unambiguous) - "ex2", + ["ex2"], [ Path("packA/ex2/script3.py"), ], @@ -154,7 +154,7 @@ def test_tmp_file_structure(input, expected, example_cases): ], ), ( # 7) copy all examples from a pack - "packA", + ["packA"], [ Path("packA/ex1/path1/script1.py"), Path("packA/ex2/script3.py"), @@ -171,7 +171,7 @@ def test_tmp_file_structure(input, expected, example_cases): ], ), ( # 9) copy all examples from all packs - "all", + ["all"], [ Path("packA/ex1/path1/script1.py"), Path("packA/ex2/script3.py"), @@ -190,7 +190,7 @@ def test_copy_examples(input, expected_paths, example_cases): examples_dir = example_cases / "case5" pm = PacksManager(root_path=examples_dir) target_dir = example_cases / "user_target" - actual = pm.copy_examples(user_input=input, target_dir=target_dir) + actual = pm.copy_examples(examples_to_copy=input, target_dir=target_dir) expected = [] for path in expected_paths: root_path = target_dir / path @@ -212,7 +212,7 @@ def test_copy_examples_location(input, expected_path, example_cases): examples_dir = example_cases / "case5" os.chdir(example_cases / "cwd") pm = PacksManager(root_path=examples_dir) - paths = pm.copy_examples(user_input="packA", target_dir=input) + paths = pm.copy_examples(examples_to_copy=["packA"], target_dir=input) actual = paths[0] expected = example_cases / expected_path assert actual == expected @@ -220,32 +220,38 @@ def test_copy_examples_location(input, expected_path, example_cases): # Test bad inputs to copy_examples on case3 # These include: -# 1) input not found (example or pack) -# 2) mixed good and bad inputs +# 1) Input not found (example or pack) +# 2) Mixed good and bad inputs # 3) Path to directory already exists +# 4) No input provided @pytest.mark.parametrize( "bad_inputs,expected,path", [ - ( - "bad_example", + ( # input not found (example or pack) + ["bad_example"], ValueError, None, - ), # input not found (example or pack) - ( + ), + ( # mixed good ex and bad inputs ["ex1", "bad_example"], ValueError, None, - ), # mixed good ex and bad inputs - ( + ), + ( # mixed good pack and bad inputs ["packA", "bad_example"], ValueError, None, - ), # mixed good pack and bad inputs - ( - "ex1", + ), + ( # path to dir already exists + ["ex1"], FileExistsError, Path("docs/examples/"), - ), # path to dir already exists + ), + ( # No input provided + [], + ValueError, + None, + ), ], ) def test_copy_examples_bad(bad_inputs, expected, path, example_cases): @@ -253,7 +259,7 @@ def test_copy_examples_bad(bad_inputs, expected, path, example_cases): pm = PacksManager(root_path=examples_dir) with pytest.raises(expected): pm.copy_examples( - user_input=bad_inputs, + examples_to_copy=bad_inputs, target_dir=examples_dir / path if path is not None else None, ) @@ -281,6 +287,6 @@ def test_copy_examples_bad_target(bad_inputs, expected, example_cases): pm = PacksManager(root_path=examples_dir) with pytest.raises(expected): pm.copy_examples( - user_input="packA", + examples_to_copy="packA", target_dir=bad_inputs, ) From 4dc153803d06224d9892aaf0c769b47c981e7644 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 09:44:23 -0400 Subject: [PATCH 22/28] rm param name from func signature --- tests/test_packsmanager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index a9d7562..8d728a3 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -190,7 +190,7 @@ def test_copy_examples(input, expected_paths, example_cases): examples_dir = example_cases / "case5" pm = PacksManager(root_path=examples_dir) target_dir = example_cases / "user_target" - actual = pm.copy_examples(examples_to_copy=input, target_dir=target_dir) + actual = pm.copy_examples(input, target_dir=target_dir) expected = [] for path in expected_paths: root_path = target_dir / path @@ -212,7 +212,7 @@ def test_copy_examples_location(input, expected_path, example_cases): examples_dir = example_cases / "case5" os.chdir(example_cases / "cwd") pm = PacksManager(root_path=examples_dir) - paths = pm.copy_examples(examples_to_copy=["packA"], target_dir=input) + paths = pm.copy_examples(["packA"], target_dir=input) actual = paths[0] expected = example_cases / expected_path assert actual == expected @@ -259,7 +259,7 @@ def test_copy_examples_bad(bad_inputs, expected, path, example_cases): pm = PacksManager(root_path=examples_dir) with pytest.raises(expected): pm.copy_examples( - examples_to_copy=bad_inputs, + bad_inputs, target_dir=examples_dir / path if path is not None else None, ) @@ -287,6 +287,6 @@ def test_copy_examples_bad_target(bad_inputs, expected, example_cases): pm = PacksManager(root_path=examples_dir) with pytest.raises(expected): pm.copy_examples( - examples_to_copy="packA", + ["packA"], target_dir=bad_inputs, ) From 5820dd39c71fe0b823069b8b4a2d71bc84e6145c Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 09:48:27 -0400 Subject: [PATCH 23/28] change scope from session to function --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4621573..5ab0d43 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,7 +22,7 @@ def tmp_examples(tmp_path_factory): yield tmp_examples -@pytest.fixture(scope="session") +@pytest.fixture(scope="function") def example_cases(tmp_path_factory): """Copy the entire examples tree into a temp directory once per test session. From 84e54a8ca11ade3725e4f7db287be6115e2e0efa Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 10:01:25 -0400 Subject: [PATCH 24/28] rglob examples after they are copied --- tests/test_packsmanager.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 8d728a3..5f77483 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -190,11 +190,9 @@ def test_copy_examples(input, expected_paths, example_cases): examples_dir = example_cases / "case5" pm = PacksManager(root_path=examples_dir) target_dir = example_cases / "user_target" - actual = pm.copy_examples(input, target_dir=target_dir) - expected = [] - for path in expected_paths: - root_path = target_dir / path - expected.append(root_path) + pm.copy_examples(input, target_dir=target_dir) + actual = list(target_dir.rglob("*")) + expected = [target_dir / path for path in expected_paths] assert actual == expected From 6c388856d8553aaa89dd0134578e17385355cc55 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 10:20:07 -0400 Subject: [PATCH 25/28] write text to the files and make check that the copied files have the same content --- tests/conftest.py | 38 +++++++++++++++++++++++++------------- tests/test_packsmanager.py | 5 +++++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5ab0d43..f83e5b6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -34,6 +34,7 @@ def example_cases(tmp_path_factory): cwd.mkdir(parents=True, exist_ok=True) user_target = root_temp_dir / "user_target" user_target.mkdir(parents=True, exist_ok=True) + # case 1: pack with no examples case1ex_dir = root_temp_dir / "case1" / "docs" / "examples" case1 = case1ex_dir / "empty_pack" # empty_pack @@ -47,13 +48,15 @@ def example_cases(tmp_path_factory): case2ex_dir / "full_pack" / "ex1" / "solution" / "diffpy-cmi" ) # full_pack, ex1 case2a.mkdir(parents=True, exist_ok=True) - (case2a / "script1.py").touch() + (case2a / "script1.py").write_text(f"# {case2a.name} script1\n") + case2b = ( case2ex_dir / "full_pack" / "ex2" / "random" / "path" ) # full_pack, ex2 case2b.mkdir(parents=True, exist_ok=True) - (case2b / "script1.py").touch() - (case2b / "script2.py").touch() + (case2b / "script1.py").write_text(f"# {case2b.name} script1\n") + (case2b / "script2.py").write_text(f"# {case2b.name} script2\n") + case2req_dir = root_temp_dir / "case2" / "requirements" / "packs" case2req_dir.mkdir(parents=True, exist_ok=True) @@ -61,16 +64,19 @@ def example_cases(tmp_path_factory): case3ex_dir = root_temp_dir / "case3" / "docs" / "examples" case3a = case3ex_dir / "packA" / "ex1" # packA, ex1 case3a.mkdir(parents=True, exist_ok=True) - (case3a / "script1.py").touch() + (case3a / "script1.py").write_text(f"# {case3a.name} script1\n") + case3b = case3ex_dir / "packA" / "ex2" / "solutions" # packA, ex2 case3b.mkdir(parents=True, exist_ok=True) - (case3b / "script2.py").touch() + (case3b / "script2.py").write_text(f"# {case3b.name} script2\n") + case3c = ( case3ex_dir / "packB" / "ex3" / "more" / "random" / "path" ) # packB, ex3 case3c.mkdir(parents=True, exist_ok=True) - (case3c / "script3.py").touch() - (case3c / "script4.py").touch() + (case3c / "script3.py").write_text(f"# {case3c.name} script3\n") + (case3c / "script4.py").write_text(f"# {case3c.name} script4\n") + case3req_dir = root_temp_dir / "case3" / "requirements" / "packs" case3req_dir.mkdir(parents=True, exist_ok=True) @@ -83,21 +89,27 @@ def example_cases(tmp_path_factory): # Case 5: multiple packs with the same example names case5ex_dir = root_temp_dir / "case5" / "docs" / "examples" + case5a = case5ex_dir / "packA" / "ex1" / "path1" # packA, ex1 case5a.mkdir(parents=True, exist_ok=True) - (case5a / "script1.py").touch() + (case5a / "script1.py").write_text(f"# {case5a.name} script1\n") + case5b = case5ex_dir / "packB" / "ex1" / "path2" # packB, ex1 case5b.mkdir(parents=True, exist_ok=True) - (case5b / "script2.py").touch() - case5c = case5ex_dir / "packA" / "ex2" # packB, ex2 + (case5b / "script2.py").write_text(f"# {case5b.name} script2\n") + + case5c = case5ex_dir / "packA" / "ex2" # packA, ex2 case5c.mkdir(parents=True, exist_ok=True) - (case5c / "script3.py").touch() + (case5c / "script3.py").write_text(f"# {case5c.name} script3\n") + case5d = case5ex_dir / "packB" / "ex3" case5d.mkdir(parents=True, exist_ok=True) - (case5d / "script4.py").touch() + (case5d / "script4.py").write_text(f"# {case5d.name} script4\n") + case5e = case5ex_dir / "packB" / "ex4" case5e.mkdir(parents=True, exist_ok=True) - (case5e / "script5.py").touch() + (case5e / "script5.py").write_text(f"# {case5e.name} script5\n") + case5req_dir = root_temp_dir / "case5" / "requirements" / "packs" case5req_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 5f77483..39136e8 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -194,6 +194,11 @@ def test_copy_examples(input, expected_paths, example_cases): actual = list(target_dir.rglob("*")) expected = [target_dir / path for path in expected_paths] assert actual == expected + for path in expected_paths: + dest = target_dir / path + source = examples_dir / path + if dest.is_file() and source.is_file(): + assert dest.read_text() == source.read_text() # Test default and targeted copy_example location on case5 From 3405e17bc1a718a3cee57785da06a3d4bb2974b7 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 10:52:09 -0400 Subject: [PATCH 26/28] check exception message rather than the exception type --- tests/test_packsmanager.py | 69 +++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 39136e8..40e9b9d 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -1,4 +1,5 @@ import os +import re from pathlib import Path import pytest @@ -230,29 +231,42 @@ def test_copy_examples_location(input, expected_path, example_cases): @pytest.mark.parametrize( "bad_inputs,expected,path", [ - ( # input not found (example or pack) + ( + # 1) Input not found (example or pack). + # Expected: Raise an error with the message. ["bad_example"], - ValueError, + "No examples or packs found for input: 'bad_example'", None, ), - ( # mixed good ex and bad inputs + ( + # 2) Mixed good example and bad input. + # Expected: Raise an error with the message. ["ex1", "bad_example"], - ValueError, + "No examples or packs found for input: 'bad_example'", None, ), - ( # mixed good pack and bad inputs + ( + # 3) Mixed good pack and bad input. + # Expected: Raise an error with the message. ["packA", "bad_example"], - ValueError, + "No examples or packs found for input: 'bad_example'", None, ), - ( # path to dir already exists + ( + # 4) Path to directory already exists. + # Expected: Raise a warning with the message. ["ex1"], - FileExistsError, + "Target directory already exists. To overwrite use --force.", Path("docs/examples/"), ), - ( # No input provided + ( + # 5) No input provided. + # Expected: Raise an error with the message. [], - ValueError, + ( + "No input specified. " + "Provide at least one example or pack to copy." + ), None, ), ], @@ -260,11 +274,9 @@ def test_copy_examples_location(input, expected_path, example_cases): def test_copy_examples_bad(bad_inputs, expected, path, example_cases): examples_dir = example_cases / "case3" pm = PacksManager(root_path=examples_dir) - with pytest.raises(expected): - pm.copy_examples( - bad_inputs, - target_dir=examples_dir / path if path is not None else None, - ) + target_dir = None if path is None else examples_dir / path + with pytest.raises(Exception, match=re.escape(expected)): + pm.copy_examples(bad_inputs, target_dir=target_dir) # Test bad target_dir to copy_examples on case3 @@ -272,24 +284,33 @@ def test_copy_examples_bad(bad_inputs, expected, path, example_cases): # 2) target is a file # 3) target is nested in a file @pytest.mark.parametrize( - "bad_inputs,expected", + "bad_target,expected", [ - (Path("nonexistent/path/target"), FileNotFoundError), # doesn't exist ( + # 1) Target doesn't exist. + # Expected: Raise an error that the target directory is missing. + Path("nonexistent/path/target"), + "Target directory does not exist", + ), + ( + # 2) Target is a file. + # Expected: Raise an error that the target path is not a directory. Path("docs/examples/packA/ex1/script4.py"), - NotADirectoryError, - ), # target is a file + "Target path is not a directory", + ), ( + # 3) Target is nested in a file. + # Expected: Raise an error that the target path is not a directory. Path("docs/examples/packA/ex1/script4.py/nested"), - NotADirectoryError, - ), # nested in file + "Target path is not a directory", + ), ], ) -def test_copy_examples_bad_target(bad_inputs, expected, example_cases): +def test_copy_examples_bad_target(bad_target, expected, example_cases): examples_dir = example_cases / "case3" pm = PacksManager(root_path=examples_dir) - with pytest.raises(expected): + with pytest.raises(Exception, match=re.escape(expected)): pm.copy_examples( ["packA"], - target_dir=bad_inputs, + target_dir=bad_target, ) From b8e433644fc93db8d9dd3be731e8227f70c17e25 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 14:42:02 -0400 Subject: [PATCH 27/28] copy_examples returns None --- src/diffpy/cmi/packsmanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diffpy/cmi/packsmanager.py b/src/diffpy/cmi/packsmanager.py index c7831d0..a924c2d 100644 --- a/src/diffpy/cmi/packsmanager.py +++ b/src/diffpy/cmi/packsmanager.py @@ -117,7 +117,7 @@ def copy_examples( self, examples_to_copy: List[str], target_dir: Path = None, - ) -> List[Path]: + ) -> None: """Copy examples or packs into the target or current working directory. From 1324185bb645d636d77241051d1b710cb2c5dc8a Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 17 Oct 2025 18:05:58 -0400 Subject: [PATCH 28/28] sort list of paths, rm bad target test --- tests/test_packsmanager.py | 64 +++++++------------------------------- 1 file changed, 11 insertions(+), 53 deletions(-) diff --git a/tests/test_packsmanager.py b/tests/test_packsmanager.py index 40e9b9d..4bce34c 100644 --- a/tests/test_packsmanager.py +++ b/tests/test_packsmanager.py @@ -192,14 +192,14 @@ def test_copy_examples(input, expected_paths, example_cases): pm = PacksManager(root_path=examples_dir) target_dir = example_cases / "user_target" pm.copy_examples(input, target_dir=target_dir) - actual = list(target_dir.rglob("*")) - expected = [target_dir / path for path in expected_paths] + actual = sorted(list(target_dir.rglob("*"))) + expected = sorted([target_dir / path for path in expected_paths]) assert actual == expected for path in expected_paths: - dest = target_dir / path - source = examples_dir / path - if dest.is_file() and source.is_file(): - assert dest.read_text() == source.read_text() + copied_path = target_dir / path + original_path = examples_dir / path + if copied_path.is_file() and original_path.is_file(): + assert copied_path.read_text() == original_path.read_text() # Test default and targeted copy_example location on case5 @@ -256,18 +256,13 @@ def test_copy_examples_location(input, expected_path, example_cases): # 4) Path to directory already exists. # Expected: Raise a warning with the message. ["ex1"], - "Target directory already exists. To overwrite use --force.", - Path("docs/examples/"), - ), - ( - # 5) No input provided. - # Expected: Raise an error with the message. - [], ( - "No input specified. " - "Provide at least one example or pack to copy." + "Example directory(ies): 'ex1' already exist. " + " Current versions of existing files have " + "been left unchanged. To overwrite, please rerun " + "and specify --force." ), - None, + Path("docs/examples/"), ), ], ) @@ -277,40 +272,3 @@ def test_copy_examples_bad(bad_inputs, expected, path, example_cases): target_dir = None if path is None else examples_dir / path with pytest.raises(Exception, match=re.escape(expected)): pm.copy_examples(bad_inputs, target_dir=target_dir) - - -# Test bad target_dir to copy_examples on case3 -# 1) target doesn't exist -# 2) target is a file -# 3) target is nested in a file -@pytest.mark.parametrize( - "bad_target,expected", - [ - ( - # 1) Target doesn't exist. - # Expected: Raise an error that the target directory is missing. - Path("nonexistent/path/target"), - "Target directory does not exist", - ), - ( - # 2) Target is a file. - # Expected: Raise an error that the target path is not a directory. - Path("docs/examples/packA/ex1/script4.py"), - "Target path is not a directory", - ), - ( - # 3) Target is nested in a file. - # Expected: Raise an error that the target path is not a directory. - Path("docs/examples/packA/ex1/script4.py/nested"), - "Target path is not a directory", - ), - ], -) -def test_copy_examples_bad_target(bad_target, expected, example_cases): - examples_dir = example_cases / "case3" - pm = PacksManager(root_path=examples_dir) - with pytest.raises(Exception, match=re.escape(expected)): - pm.copy_examples( - ["packA"], - target_dir=bad_target, - )