From 5b8839a1bc63a836bb9ede596b2a366afccfd346 Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 07:57:05 -0400 Subject: [PATCH 1/7] cleaning up conftest.py, getting rid of os --- src/diffpy/labpdfproc/tests/conftest.py | 36 +++++++++++++---------- src/diffpy/labpdfproc/tests/test_tools.py | 3 ++ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/diffpy/labpdfproc/tests/conftest.py b/src/diffpy/labpdfproc/tests/conftest.py index 1e4ab40..5b8c323 100644 --- a/src/diffpy/labpdfproc/tests/conftest.py +++ b/src/diffpy/labpdfproc/tests/conftest.py @@ -1,4 +1,3 @@ -import os from pathlib import Path import pytest @@ -6,37 +5,42 @@ @pytest.fixture def user_filesystem(tmp_path): - directory = Path(tmp_path) - os.chdir(directory) - - input_dir = Path(tmp_path).resolve() / "input_dir" + base_dir = Path(tmp_path) + input_dir = base_dir / "input_dir" input_dir.mkdir(parents=True, exist_ok=True) chi_data = "dataformat = twotheta\n mode = xray\n # chi_Q chi_I\n 1 2\n 3 4\n 5 6\n 7 8\n" xy_data = "1 2\n 3 4\n 5 6\n 7 8" - unreadable_data = "This is an unreadable file." + unreadable_data = "This is a file with no data that is non-readable by " "LoadData" binary_data = b"\x00\x01\x02\x03\x04" - with open("good_data.chi", "w") as f: + with open(base_dir / "good_data.chi", "w") as f: f.write(chi_data) - with open("good_data.xy", "w") as f: + with open(base_dir / "good_data.xy", "w") as f: f.write(xy_data) - with open("good_data.txt", "w") as f: + with open(base_dir / "good_data.txt", "w") as f: f.write(chi_data) - with open("unreadable_file.txt", "w") as f: + with open(base_dir / "unreadable_file.txt", "w") as f: f.write(unreadable_data) - with open("binary.pkl", "wb") as f: + with open(base_dir / "binary.pkl", "wb") as f: f.write(binary_data) - with open(os.path.join(input_dir, "good_data.chi"), "w") as f: + with open(input_dir / "good_data.chi", "w") as f: f.write(chi_data) - with open(os.path.join(input_dir, "good_data.xy"), "w") as f: + with open(input_dir / "good_data.xy", "w") as f: f.write(xy_data) - with open(os.path.join(input_dir, "good_data.txt"), "w") as f: + with open(input_dir / "good_data.txt", "w") as f: f.write(chi_data) - with open(os.path.join(input_dir, "unreadable_file.txt"), "w") as f: + with open(input_dir / "unreadable_file.txt", "w") as f: f.write(unreadable_data) - with open(os.path.join(input_dir, "binary.pkl"), "wb") as f: + with open(input_dir / "binary.pkl", "wb") as f: f.write(binary_data) + with open(input_dir / "file_list.txt", "w") as f: + f.write("good_data.chi \n good_data.xy \n good_data.txt \n missing_file.txt") + with open(input_dir / "file_list_example2.txt", "w") as f: + f.write("input_dir/good_data.chi \n") + f.write("good_data.xy \n") + f.write(f"{str(input_dir.resolve() / 'good_data.txt')}\n") + yield tmp_path diff --git a/src/diffpy/labpdfproc/tests/test_tools.py b/src/diffpy/labpdfproc/tests/test_tools.py index 10491ac..6e564c5 100644 --- a/src/diffpy/labpdfproc/tests/test_tools.py +++ b/src/diffpy/labpdfproc/tests/test_tools.py @@ -1,3 +1,4 @@ +import os import re from pathlib import Path @@ -16,6 +17,7 @@ @pytest.mark.parametrize("inputs, expected", params1) def test_set_output_directory(inputs, expected, user_filesystem): + os.chdir(user_filesystem) expected_output_directory = Path(user_filesystem) / expected[0] cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) @@ -26,6 +28,7 @@ def test_set_output_directory(inputs, expected, user_filesystem): def test_set_output_directory_bad(user_filesystem): + os.chdir(user_filesystem) cli_inputs = ["2.5", "--output-directory", "good_data.chi"] actual_args = get_args(cli_inputs) with pytest.raises(FileExistsError): From ab8f7194f72dee91edb90920226b7c18d1dc4f2d Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 08:43:21 -0400 Subject: [PATCH 2/7] move test for whether test files are readble or not to conftest --- src/diffpy/labpdfproc/tests/test_fixtures.py | 18 ++++ src/diffpy/labpdfproc/tests/test_tools.py | 101 ++++++++++++++++++- 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/diffpy/labpdfproc/tests/test_fixtures.py diff --git a/src/diffpy/labpdfproc/tests/test_fixtures.py b/src/diffpy/labpdfproc/tests/test_fixtures.py new file mode 100644 index 0000000..8107bb1 --- /dev/null +++ b/src/diffpy/labpdfproc/tests/test_fixtures.py @@ -0,0 +1,18 @@ +import os + +import pytest + +from diffpy.utils.parsers import loadData + + +# Test that our readable and unreadable files are indeed readable and +# unreadable by loadData (which is our definition of readable and unreadable) +def test_loadData_with_input_files(user_filesystem): + os.chdir(user_filesystem) + xarray_chi, yarray_chi = loadData("good_data.chi", unpack=True) + xarray_xy, yarray_xy = loadData("good_data.xy", unpack=True) + xarray_txt, yarray_txt = loadData("good_data.txt", unpack=True) + with pytest.raises(ValueError): + xarray_txt, yarray_txt = loadData("unreadable_file.txt", unpack=True) + with pytest.raises(ValueError): + xarray_pkl, yarray_pkl = loadData("binary.pkl", unpack=True) diff --git a/src/diffpy/labpdfproc/tests/test_tools.py b/src/diffpy/labpdfproc/tests/test_tools.py index dbd6a99..c2046f0 100644 --- a/src/diffpy/labpdfproc/tests/test_tools.py +++ b/src/diffpy/labpdfproc/tests/test_tools.py @@ -5,7 +5,106 @@ import pytest from diffpy.labpdfproc.labpdfprocapp import get_args -from diffpy.labpdfproc.tools import known_sources, load_user_metadata, set_output_directory, set_wavelength +from diffpy.labpdfproc.tools import ( + known_sources, + load_user_metadata, + set_input_files, + set_output_directory, + set_wavelength, +) + +# Use cases can be found here: https://github.com/diffpy/diffpy.labpdfproc/issues/48 + +# This test covers existing single input file, directory, a file list, and multiple files +# We store absolute path into input_directory and file names into input_file +params_input = [ + (["good_data.chi"], ["good_data.chi"]), # single good file, same directory + (["input_dir/good_data.chi"], ["input_dir/good_data.chi"]), # single good file, input directory + ( # glob current directory + ["."], + ["good_data.chi", "good_data.xy", "good_data.txt", "unreadable_file.txt", "binary.pkl"], + ), + ( # glob input directory + ["./input_dir"], + [ + "input_dir/good_data.chi", + "input_dir/good_data.xy", + "input_dir/good_data.txt", + "input_dir/unreadable_file.txt", + "input_dir/binary.pkl", + ], + ), + ( # list of files provided (we skip if encountering invalid files) + ["good_data.chi", "good_data.xy", "unreadable_file.txt", "missing_file.txt"], + ["good_data.chi", "good_data.xy", "unreadable_file.txt"], + ), + ( # list of files provided (with invalid files and files in different directories) + ["input_dir/good_data.chi", "good_data.chi", "missing_file.txt"], + ["input_dir/good_data.chi", "good_data.chi"], + ), + ( # file_list.txt list of files provided + ["file_list_dir/file_list.txt"], + ["good_data.chi", "good_data.xy", "good_data.txt"], + ), + ( # file_list_example2.txt list of files provided in different directories + ["file_list_dir/file_list_example2.txt"], + ["input_dir/good_data.chi", "good_data.xy", "input_dir/good_data.txt"], + ), +] + + +@pytest.mark.parametrize("inputs, expected", params_input) +def test_set_input_files(inputs, expected, user_filesystem): + expected_input_directory = [] + for expected_path in expected: + expected_input_directory.append(Path(user_filesystem) / expected_path) + + cli_inputs = ["2.5"] + inputs + actual_args = get_args(cli_inputs) + actual_args = set_input_files(actual_args) + assert set(actual_args.input_directory) == set(expected_input_directory) + + +# This test is for existing single input file or directory absolute path not in cwd +# Here we are in user_filesystem/input_dir, testing for a file or directory in user_filesystem +params_input_not_cwd = [ + (["good_data.chi"], ["good_data.chi"]), + (["."], ["good_data.chi", "good_data.xy", "good_data.txt", "unreadable_file.txt", "binary.pkl"]), +] + + +@pytest.mark.parametrize("inputs, expected", params_input_not_cwd) +def test_set_input_files_not_cwd(inputs, expected, user_filesystem): + expected_input_directory = [] + for expected_path in expected: + expected_input_directory.append(Path(user_filesystem) / expected_path) + actual_input = [str(Path(user_filesystem) / inputs[0])] + os.chdir("input_dir") + + cli_inputs = ["2.5"] + actual_input + actual_args = get_args(cli_inputs) + actual_args = set_input_files(actual_args) + assert set(actual_args.input_directory) == set(expected_input_directory) + + +# This test covers non-existing single input file or directory, in this case we raise an error with message +params_input_bad = [ + (["non_existing_file.xy"], "Please specify at least one valid input file or directory."), + (["./input_dir/non_existing_file.xy"], "Please specify at least one valid input file or directory."), + (["./non_existing_dir"], "Please specify at least one valid input file or directory."), +] + + +@pytest.mark.parametrize("inputs, msg", params_input_bad) +def test_set_input_files_bad(inputs, msg, user_filesystem): + cli_inputs = ["2.5"] + inputs + actual_args = get_args(cli_inputs) + with pytest.raises(ValueError, match=msg[0]): + actual_args = set_input_files(actual_args) + + +# Pass files to loadData and use it to check if file is valid or not + params1 = [ ([], ["."]), From 1643f8d7c59b9332d57e5017816bc8bb546d9670 Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 09:04:50 -0400 Subject: [PATCH 3/7] finalize tests --- src/diffpy/labpdfproc/tests/test_tools.py | 39 ++++++++++++++++------- src/diffpy/labpdfproc/tools.py | 4 +++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/diffpy/labpdfproc/tests/test_tools.py b/src/diffpy/labpdfproc/tests/test_tools.py index c2046f0..19ab4ed 100644 --- a/src/diffpy/labpdfproc/tests/test_tools.py +++ b/src/diffpy/labpdfproc/tests/test_tools.py @@ -8,7 +8,7 @@ from diffpy.labpdfproc.tools import ( known_sources, load_user_metadata, - set_input_files, + set_input_lists, set_output_directory, set_wavelength, ) @@ -34,7 +34,22 @@ "input_dir/binary.pkl", ], ), - ( # list of files provided (we skip if encountering invalid files) + ( # glob list of input directories + [".", "./input_dir"], + [ + "./good_data.chi", + "./good_data.xy", + "./good_data.txt", + "./unreadable_file.txt", + "./binary.pkl", + "input_dir/good_data.chi", + "input_dir/good_data.xy", + "input_dir/good_data.txt", + "input_dir/unreadable_file.txt", + "input_dir/binary.pkl", + ], + ), + ( # list of files provided (we skip if encountering missing files) ["good_data.chi", "good_data.xy", "unreadable_file.txt", "missing_file.txt"], ["good_data.chi", "good_data.xy", "unreadable_file.txt"], ), @@ -43,26 +58,26 @@ ["input_dir/good_data.chi", "good_data.chi"], ), ( # file_list.txt list of files provided - ["file_list_dir/file_list.txt"], + ["input_dir/file_list.txt"], ["good_data.chi", "good_data.xy", "good_data.txt"], ), ( # file_list_example2.txt list of files provided in different directories - ["file_list_dir/file_list_example2.txt"], + ["input_dir/file_list_example2.txt"], ["input_dir/good_data.chi", "good_data.xy", "input_dir/good_data.txt"], ), ] @pytest.mark.parametrize("inputs, expected", params_input) -def test_set_input_files(inputs, expected, user_filesystem): - expected_input_directory = [] - for expected_path in expected: - expected_input_directory.append(Path(user_filesystem) / expected_path) +def test_set_input_lists(inputs, expected, user_filesystem): + base_dir = Path(user_filesystem) + os.chdir(base_dir) + expected_paths = [Path(user_filesystem).resolve() / expected_path for expected_path in expected] cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) - actual_args = set_input_files(actual_args) - assert set(actual_args.input_directory) == set(expected_input_directory) + actual_args = set_input_lists(actual_args) + assert actual_args.input_directory == expected_paths # This test is for existing single input file or directory absolute path not in cwd @@ -83,7 +98,7 @@ def test_set_input_files_not_cwd(inputs, expected, user_filesystem): cli_inputs = ["2.5"] + actual_input actual_args = get_args(cli_inputs) - actual_args = set_input_files(actual_args) + actual_args = set_input_lists(actual_args) assert set(actual_args.input_directory) == set(expected_input_directory) @@ -100,7 +115,7 @@ def test_set_input_files_bad(inputs, msg, user_filesystem): cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) with pytest.raises(ValueError, match=msg[0]): - actual_args = set_input_files(actual_args) + actual_args = set_input_lists(actual_args) # Pass files to loadData and use it to check if file is valid or not diff --git a/src/diffpy/labpdfproc/tools.py b/src/diffpy/labpdfproc/tools.py index caa012d..4ac1223 100644 --- a/src/diffpy/labpdfproc/tools.py +++ b/src/diffpy/labpdfproc/tools.py @@ -28,6 +28,10 @@ def set_output_directory(args): return output_dir +def set_input_lists(args): + return args + + def set_wavelength(args): """ Set the wavelength based on the given input arguments From 8285277d7b449a86b31dd72db157ac4d67fcf8b4 Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 09:09:05 -0400 Subject: [PATCH 4/7] finalizing all the tests, not just the good ones --- src/diffpy/labpdfproc/tests/test_tools.py | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/diffpy/labpdfproc/tests/test_tools.py b/src/diffpy/labpdfproc/tests/test_tools.py index 19ab4ed..42737a8 100644 --- a/src/diffpy/labpdfproc/tests/test_tools.py +++ b/src/diffpy/labpdfproc/tests/test_tools.py @@ -72,7 +72,7 @@ def test_set_input_lists(inputs, expected, user_filesystem): base_dir = Path(user_filesystem) os.chdir(base_dir) - expected_paths = [Path(user_filesystem).resolve() / expected_path for expected_path in expected] + expected_paths = [base_dir.resolve() / expected_path for expected_path in expected] cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) @@ -88,20 +88,6 @@ def test_set_input_lists(inputs, expected, user_filesystem): ] -@pytest.mark.parametrize("inputs, expected", params_input_not_cwd) -def test_set_input_files_not_cwd(inputs, expected, user_filesystem): - expected_input_directory = [] - for expected_path in expected: - expected_input_directory.append(Path(user_filesystem) / expected_path) - actual_input = [str(Path(user_filesystem) / inputs[0])] - os.chdir("input_dir") - - cli_inputs = ["2.5"] + actual_input - actual_args = get_args(cli_inputs) - actual_args = set_input_lists(actual_args) - assert set(actual_args.input_directory) == set(expected_input_directory) - - # This test covers non-existing single input file or directory, in this case we raise an error with message params_input_bad = [ (["non_existing_file.xy"], "Please specify at least one valid input file or directory."), @@ -112,15 +98,14 @@ def test_set_input_files_not_cwd(inputs, expected, user_filesystem): @pytest.mark.parametrize("inputs, msg", params_input_bad) def test_set_input_files_bad(inputs, msg, user_filesystem): + base_dir = Path(user_filesystem) + os.chdir(base_dir) cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) with pytest.raises(ValueError, match=msg[0]): actual_args = set_input_lists(actual_args) -# Pass files to loadData and use it to check if file is valid or not - - params1 = [ ([], ["."]), (["--output-directory", "."], ["."]), From 7e687891c437cdf7b5196c36494584d763b2afde Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 09:40:10 -0400 Subject: [PATCH 5/7] all tests passing, much simplified. Cannot yet handle lists from file --- src/diffpy/labpdfproc/tests/test_tools.py | 36 ++++++++++------------- src/diffpy/labpdfproc/tools.py | 32 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/diffpy/labpdfproc/tests/test_tools.py b/src/diffpy/labpdfproc/tests/test_tools.py index 42737a8..996a0c6 100644 --- a/src/diffpy/labpdfproc/tests/test_tools.py +++ b/src/diffpy/labpdfproc/tests/test_tools.py @@ -49,14 +49,6 @@ "input_dir/binary.pkl", ], ), - ( # list of files provided (we skip if encountering missing files) - ["good_data.chi", "good_data.xy", "unreadable_file.txt", "missing_file.txt"], - ["good_data.chi", "good_data.xy", "unreadable_file.txt"], - ), - ( # list of files provided (with invalid files and files in different directories) - ["input_dir/good_data.chi", "good_data.chi", "missing_file.txt"], - ["input_dir/good_data.chi", "good_data.chi"], - ), ( # file_list.txt list of files provided ["input_dir/file_list.txt"], ["good_data.chi", "good_data.xy", "good_data.txt"], @@ -77,22 +69,24 @@ def test_set_input_lists(inputs, expected, user_filesystem): cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) actual_args = set_input_lists(actual_args) - assert actual_args.input_directory == expected_paths - - -# This test is for existing single input file or directory absolute path not in cwd -# Here we are in user_filesystem/input_dir, testing for a file or directory in user_filesystem -params_input_not_cwd = [ - (["good_data.chi"], ["good_data.chi"]), - (["."], ["good_data.chi", "good_data.xy", "good_data.txt", "unreadable_file.txt", "binary.pkl"]), -] + assert list(actual_args.input_directory).sort() == expected_paths.sort() # This test covers non-existing single input file or directory, in this case we raise an error with message params_input_bad = [ - (["non_existing_file.xy"], "Please specify at least one valid input file or directory."), - (["./input_dir/non_existing_file.xy"], "Please specify at least one valid input file or directory."), - (["./non_existing_dir"], "Please specify at least one valid input file or directory."), + ( + ["non_existing_file.xy"], + "Cannot find non_existing_file.xy. Please specify valid input file(s) or directories.", + ), + ( + ["./input_dir/non_existing_file.xy"], + "Cannot find ./input_dir/non_existing_file.xy. Please specify valid input file(s) or directories.", + ), + (["./non_existing_dir"], "Cannot find ./non_existing_dir. Please specify valid input file(s) or directories."), + ( # list of files provided (with missing files) + ["good_data.chi", "good_data.xy", "unreadable_file.txt", "missing_file.txt"], + "Cannot find missing_file.txt. Please specify valid input file(s) or directories.", + ), ] @@ -102,7 +96,7 @@ def test_set_input_files_bad(inputs, msg, user_filesystem): os.chdir(base_dir) cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) - with pytest.raises(ValueError, match=msg[0]): + with pytest.raises(FileNotFoundError, match=msg[0]): actual_args = set_input_lists(actual_args) diff --git a/src/diffpy/labpdfproc/tools.py b/src/diffpy/labpdfproc/tools.py index 4ac1223..83ac075 100644 --- a/src/diffpy/labpdfproc/tools.py +++ b/src/diffpy/labpdfproc/tools.py @@ -29,6 +29,38 @@ def set_output_directory(args): def set_input_lists(args): + """ + Set input directory and files. + + It takes cli inputs, checks if they are files or directories and creates + a list of files to be processed which is stored in the args Namespace. + + Parameters + ---------- + args argparse.Namespace + the arguments from the parser + + Returns + ------- + args argparse.Namespace + + """ + + input_paths = [] + for input in args.input: + input_path = Path(input).resolve() + if input_path.exists(): + if input_path.is_file(): + input_paths.append(input_path) + elif input_path.is_dir(): + input_files = input_path.glob("*") + input_files = [file.resolve() for file in input_files if file.is_file()] + input_paths.extend(input_files) + else: + raise FileNotFoundError(f"Cannot find {input}. Please specify valid input file(s) or directories.") + else: + raise FileNotFoundError(f"Cannot find {input}") + setattr(args, "input_directory", input_paths) return args From 9928c604da5a9dd1d96b76c0729dcab844a1942b Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 09:43:58 -0400 Subject: [PATCH 6/7] rename the list of files that is generated --- src/diffpy/labpdfproc/tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diffpy/labpdfproc/tools.py b/src/diffpy/labpdfproc/tools.py index 83ac075..2bf2379 100644 --- a/src/diffpy/labpdfproc/tools.py +++ b/src/diffpy/labpdfproc/tools.py @@ -60,7 +60,7 @@ def set_input_lists(args): raise FileNotFoundError(f"Cannot find {input}. Please specify valid input file(s) or directories.") else: raise FileNotFoundError(f"Cannot find {input}") - setattr(args, "input_directory", input_paths) + setattr(args, "input_paths", input_paths) return args From 17df8480ed34cd6800b7d7083225d5c39766bc66 Mon Sep 17 00:00:00 2001 From: Simon Billinge Date: Fri, 10 May 2024 09:49:56 -0400 Subject: [PATCH 7/7] complete the change of the args attribute to input_paths --- src/diffpy/labpdfproc/tests/test_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diffpy/labpdfproc/tests/test_tools.py b/src/diffpy/labpdfproc/tests/test_tools.py index 996a0c6..db75965 100644 --- a/src/diffpy/labpdfproc/tests/test_tools.py +++ b/src/diffpy/labpdfproc/tests/test_tools.py @@ -69,7 +69,7 @@ def test_set_input_lists(inputs, expected, user_filesystem): cli_inputs = ["2.5"] + inputs actual_args = get_args(cli_inputs) actual_args = set_input_lists(actual_args) - assert list(actual_args.input_directory).sort() == expected_paths.sort() + assert list(actual_args.input_paths).sort() == expected_paths.sort() # This test covers non-existing single input file or directory, in this case we raise an error with message