Refactor: Improve how FileIO and FileCompare are invoked and interact

.github/workflows/ci.yml

@@ -1,8 +1,7 @@
 name: Run PyTest on Push
 
 on:
-  push:
-    branches: [ "*" ]
+  push
 
 jobs:
   build:

README.md

@@ -128,13 +128,13 @@ Metadata lines are formatted as comments starting with two hashes; these are wri
 
 # List of Options
 - `-h`, `--help`: show this help message and exit
-- `-r`, `--ref_table`: Reference tabular dataset.
-- `-q`, `--query_table`: Subject tabular dataset to compare.
-- `-rs`, `--ref_sampleid_key`: The column/field name that contains sample IDs in the reference table.
-- `-qs`, `--query_sampleid_key`: The column/field name that contains sample IDs in the query table.
+- `-r`, `--ref_table`: Reference tabular dataset. [REQUIRED: TRUE]
+- `-q`, `--query_table`: Subject tabular dataset to compare. [REQUIRED: TRUE]
+- `-rs`, `--ref_sampleid_key`: The column/field name that contains sample IDs in the reference table. [REQUIRED: TRUE]
+- `-qs`, `--query_sampleid_key`: The column/field name that contains sample IDs in the query table. [OPTIONAL] [DEFAULT: same as provided value for `--ref_sampleid_key`]
 - `-e`, `--strict`: Whether to raise warnings as exceptions or not. **NB** Using this option will cause the program to quit upon encountering differences in the datasets and consequenctly, no output files will be generated. Ordinarily, this flag should never be used.
-- `-m`, `--mode`: How to execute comparison. [quick_compare, compare_subset]
+- `-m`, `--mode`: How to execute comparison. [VALID: quick_compare, compare_subset] [DEFAULT: quick_compare]
 - `-l`, `--field_list`: Comma-separated list of column names/fields to compare, required and used ONLY if using mode 'compare_subset'.
 - `-c`, `--custom_config`: Custom config file containing col_name and value mappings in .yaml format.
-- `-o`, `--output_filename`: Path to output file to be written.
-- `-f`, `--write_format`: File format to write comparison output to ['json', 'csv'].
+- `-o`, `--output_filename`: Path to output file to be written. [REQUIRED: TRUE]
+- `-f`, `--write_format`: File format to write comparison output to. [VALID: 'csv', 'json']

tests/table_validator_tests/test_file_compare.py

@@ -12,90 +12,79 @@
 
 @pytest.fixture
 def test_init():
-    left_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
-    right_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_json.json", sample_id_key="sample_id")
 
-    test_object = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader)
+    test_object = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "ref_toy_json.json", l_sample_id_key="sample_id")
     assert test_object, "A test file_compare object should be built"
     return test_object
 
 @pytest.fixture
 def test_init_config():
-    my_ref_fileio_object = File_IO(input_file= TEST_DATA_DIR / "data_map_test_left.csv", sample_id_key="col_left_1", config= TEST_DATA_DIR / "sample_config.yaml")
-    my_ref_fileio_object_no_config = File_IO(input_file= TEST_DATA_DIR / "data_map_test_left.csv", sample_id_key="col_left_1")
-    my_query_fileio_object = File_IO(input_file= TEST_DATA_DIR / "data_map_test_right.csv", sample_id_key="col_right_1")
 
-    test_object_no_config = FileCompare(left_data=my_ref_fileio_object_no_config.data_reader, right_data=my_query_fileio_object.data_reader)
-    test_object_w_config = FileCompare(left_data=my_ref_fileio_object.data_reader, right_data=my_query_fileio_object.data_reader)
+    test_object_no_config = FileCompare(left_file=TEST_DATA_DIR / "data_map_test_left.csv", right_file=TEST_DATA_DIR / "data_map_test_right.csv", l_sample_id_key="col_left_1", r_sample_id_key="col_right_1")
+    test_object_w_config = FileCompare(left_file=TEST_DATA_DIR / "data_map_test_left.csv", right_file=TEST_DATA_DIR / "data_map_test_right.csv", l_sample_id_key="col_left_1", r_sample_id_key="col_right_1", config=TEST_DATA_DIR / "sample_config.yaml")
 
     return test_object_no_config, test_object_w_config
 
 def test_create_schema(test_init):
     test_object = test_init
-    test_object.create_schema()
+    test_object._create_schema()
 
     assert len(test_object.sample_schema.keys()) == 4, "There should be 4 fields in the schema"
     assert test_object.sample_schema["sample_id"]["valid_types"] == test_object.sample_schema["field1"]["valid_types"] == set([str])
 
 def test_confirm_schema(test_init):
     test_object = test_init
-    test_object.create_schema()
-    test_object.confirm_schema()
+    test_object._create_schema()
+    test_object._confirm_schema()
     assert test_object.schema_errors["sample_1"] == {"sample_id": "SchemaMatch", "field1": "SchemaMatch", "field2": "SchemaMatch", "field3": "SchemaMatch"}
 
 def test_compare_values(test_init):
     test_object = test_init
-    test_object.create_schema()
     test_object.compare_values()
     assert test_object.value_mismatches["sample_1"] == {"sample_id": "ValueMatch", "field1": "ValueMatch", "field2": "ValueMatch", "field3": "ValueMatch"}
 
 def test_confirm_schema_fail():
-    left_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
-    right_data = File_IO(input_file= TEST_DATA_DIR / "query_toy_json_schemamismatch.json", sample_id_key="sample_id")
-    test_object_lenient = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader)
-    test_object_lenient.create_schema()
+    test_object_lenient = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "query_toy_json_schemamismatch.json", l_sample_id_key="sample_id")
+    test_object_lenient._create_schema()
 
     ## there should be a schema check failure here
     with pytest.warns(SchemaMismatchWarning):
-        test_object_lenient.confirm_schema()
+        test_object_lenient._confirm_schema()
 
     ## using strict mode, this warning should now be an exception -- pytest.raises needed to catch it
-    test_object_strict = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader, strict=True)
-    test_object_strict.create_schema()
+    test_object_strict = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "query_toy_json_schemamismatch.json", l_sample_id_key="sample_id", strict=True)
+    test_object_strict._create_schema()
     with pytest.raises(SchemaMismatchWarning) as error_info:
-        test_object_strict.confirm_schema()
+        test_object_strict._confirm_schema()
 
 def test_compare_values_fail():
-    left_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
-    right_data = File_IO(input_file= TEST_DATA_DIR / "query_toy_json_valmismatch.json", sample_id_key="sample_id")
-    test_object = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader)
-    test_object.create_schema()
-    test_object.confirm_schema()
+
+    test_object = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "query_toy_json_valmismatch.json", l_sample_id_key="sample_id")
+    test_object._create_schema()
+    test_object._confirm_schema()
     ## there should be a value mismatch flagged here
     with pytest.warns(ValueMismatchWarning, match="Mismatch in value of field field2 for sample sample_4. Expected: 42 Found: fourtwo"):
         test_object.compare_values()
 
 def test_compare_values_specific_fields():
-    left_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
-    right_data = File_IO(input_file= TEST_DATA_DIR / "query_toy_json_valmismatch.json", sample_id_key="sample_id")
-    test_object = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader)
-    test_object.create_schema()
-    test_object.confirm_schema()
+
+    test_object = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "query_toy_json_valmismatch.json", l_sample_id_key="sample_id")
+    test_object._create_schema()
+    test_object._confirm_schema()
     ## even though this test uses a value_mismatch test fixture
     ## there should be no warnings as the field in question is not included in the provided list of field names to check
     test_object.compare_values(field_names=["sample_id", "field1", "field3"])
     assert test_object.value_mismatches["sample_1"] == {"sample_id": "ValueMatch", "field1": "ValueMatch", "field2": "Field not checked", "field3": "ValueMatch"}
 
 def test_compare_fail_with_no_config(test_init_config):
     test_object_no_config, _ = test_init_config
-    test_object_no_config.create_schema()
+    test_object_no_config._create_schema()
     with pytest.raises(Exception):
-        test_object_no_config.confirm_schema()
+        test_object_no_config._confirm_schema()
 
 def test_compare_values_config(test_init_config):
     _, test_object_w_config = test_init_config
-    test_object_w_config.create_schema()
-    test_object_w_config.confirm_schema()
+
     test_object_w_config.compare_values()
     assert test_object_w_config.schema_errors["sample1"] == {"sample_id": "sample1", "col_right_1": "SchemaMatch", "col_right_2": "SchemaMatch", "col_right_3": "SchemaMatch"}
     assert test_object_w_config.value_mismatches["sample1"] == {"sample_id": "sample1", "col_right_1": "ValueMatch", "col_right_2": "ValueMatch", "col_right_3": "ValueMatch"}
@@ -118,8 +107,7 @@ def _files_are_identical(file1, file2):
 
 def test_output_files_json(test_init, tmp_path):
     test_object = test_init
-    test_object.create_schema()
-    test_object.confirm_schema()
+
     test_object.compare_values()
     metadata = {
                 "prog": "TableValidator_pytest",
@@ -131,8 +119,7 @@ def test_output_files_json(test_init, tmp_path):
 
 def test_output_files_csv(test_init, tmp_path):
     test_object = test_init
-    test_object.create_schema()
-    test_object.confirm_schema()
+
     test_object.compare_values()
     metadata = {
                 "prog": "TableValidator_pytest",
@@ -144,23 +131,20 @@ def test_output_files_csv(test_init, tmp_path):
     assert _files_are_identical(file1= TEST_DATA_DIR / "pytest_out_all_match_value_check.csv", file2=f"{tmp_path}/test_out_all_match_value_check.csv")[0], "Files should be identical!"
 
 def test_output_files_schema_mismatch():
-    left_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
-    right_data = File_IO(input_file= TEST_DATA_DIR / "query_toy_json_schemamismatch.json", sample_id_key="sample_id")
-    test_object_lenient = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader)
-    test_object_lenient.create_schema()
+
+    test_object_lenient = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "query_toy_json_schemamismatch.json", l_sample_id_key="sample_id")
+    test_object_lenient._create_schema()
 
     ## there should be a schema check failure here
     with pytest.warns(SchemaMismatchWarning):
-        test_object_lenient.confirm_schema()
+        test_object_lenient._confirm_schema()
 
     assert test_object_lenient.schema_errors["sample_4"]["field2"] in ["SchemaMismatch. Expected {<class 'int'>, <class 'str'>} Found <class 'bool'>", "SchemaMismatch. Expected {<class 'str'>, <class 'int'>} Found <class 'bool'>"]
 
 def test_output_files_value_mismatch(tmp_path):
-    left_data = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
-    right_data = File_IO(input_file= TEST_DATA_DIR / "query_toy_json_valmismatch.json", sample_id_key="sample_id")
-    test_object = FileCompare(left_data=left_data.data_reader, right_data=right_data.data_reader)
-    test_object.create_schema()
-    test_object.confirm_schema()
+
+    test_object = FileCompare(left_file=TEST_DATA_DIR / "ref_toy_csv.csv", right_file=TEST_DATA_DIR / "query_toy_json_valmismatch.json", l_sample_id_key="sample_id")
+
     ## there should be a value mismatch flagged here
     with pytest.warns(ValueMismatchWarning, match="Mismatch in value of field field2 for sample sample_4. Expected: 42 Found: fourtwo"):
         test_object.compare_values()

tests/table_validator_tests/test_file_io.py

@@ -10,28 +10,28 @@
 def test_init():
     my_fileio_object = File_IO(input_file= TEST_DATA_DIR / "ref_toy_csv.csv", sample_id_key="sample_id")
     assert my_fileio_object.input_file_type == "csv", "Wrong file type found"
-    assert len(my_fileio_object.data_reader.keys()) == 5, "Expected 5 sample_ids in the test data"
+    assert len(my_fileio_object.data_dict.keys()) == 5, "Expected 5 sample_ids in the test data"
     assert len(my_fileio_object.fields) == 4, f"Expected 4 columns/fields in data"
     assert sorted(my_fileio_object.fields) == sorted(["sample_id", "field1", "field2", "field3"]), "Did not find all expected fields"
 
 def test_read_from_tsv():
     my_fileio_object = File_IO(input_file= TEST_DATA_DIR / "ref_toy_tsv.tsv", sample_id_key="sample_id")
     assert my_fileio_object.input_file_type == "tsv", "Wrong file type found"
-    assert len(my_fileio_object.data_reader.keys()) == 5, "Expected 5 sample_ids in the test data"
+    assert len(my_fileio_object.data_dict.keys()) == 5, "Expected 5 sample_ids in the test data"
     assert len(my_fileio_object.fields) == 4, f"Expected 4 columns/fields in data"
     assert sorted(my_fileio_object.fields) == sorted(["sample_id", "field1", "field2", "field3"]), "Did not find all expected fields"
 
 def test_read_from_json():
     my_fileio_object = File_IO(input_file= TEST_DATA_DIR / "ref_toy_json.json", sample_id_key="sample_id")
     assert my_fileio_object.input_file_type == "json", "Wrong file type found"
-    assert len(my_fileio_object.data_reader.keys()) == 5, "Expected 5 sample_ids in the test data"
+    assert len(my_fileio_object.data_dict.keys()) == 5, "Expected 5 sample_ids in the test data"
     assert len(my_fileio_object.fields) == 4, f"Expected 4 columns/fields in data"
     assert sorted(my_fileio_object.fields) == sorted(["sample_id", "field1", "field2", "field3"]), "Did not find all expected fields"
 
 def test_read_from_yaml():
     my_fileio_object = File_IO(input_file= TEST_DATA_DIR / "ref_toy_yaml.yaml", sample_id_key="sample_id")
     assert my_fileio_object.input_file_type == "yaml", "Wrong file type found"
-    assert len(my_fileio_object.data_reader.keys()) == 5, "Expected 5 sample_ids in the test data"
+    assert len(my_fileio_object.data_dict.keys()) == 5, "Expected 5 sample_ids in the test data"
     assert len(my_fileio_object.fields) == 4, f"Expected 4 columns/fields in data"
     assert sorted(my_fileio_object.fields) == sorted(["sample_id", "field1", "field2", "field3"]), "Did not find all expected fields"
 
@@ -72,6 +72,6 @@ def test_data_mapping():
     my_ref_fileio_object_no_config = File_IO(input_file= TEST_DATA_DIR / "data_map_test_left.csv", sample_id_key="col_left_1")
     my_query_fileio_object = File_IO(input_file= TEST_DATA_DIR / "data_map_test_right.csv", sample_id_key="col_right_1")
 
-    assert list(my_ref_fileio_object.data_reader.keys()) == list(my_query_fileio_object.data_reader.keys()), "Config mapping erroneously altered sample_ids"
+    assert list(my_ref_fileio_object.data_dict.keys()) == list(my_query_fileio_object.data_dict.keys()), "Config mapping erroneously altered sample_ids"
     assert sorted(my_ref_fileio_object.fields) == sorted(my_query_fileio_object.fields), "Config mapping on column names went wrong"
     assert not sorted(my_ref_fileio_object_no_config.fields) == sorted(my_query_fileio_object.fields), "Column names should not match when config is not used"

validation_toolkit/TableValidator/file_compare.py

@@ -1,27 +1,46 @@
 import warnings
 import json
 from collections import defaultdict
+from validation_toolkit.TableValidator.file_io import File_IO
 from validation_toolkit.TableValidator.custom_exceptions import KeyMismatchWarning, SchemaMismatchWarning, FieldMismatchWarning, ValueMismatchWarning, MissingSampleWarning
 
 class FileCompare():
-    def __init__(self, left_data: dict, right_data: dict = None, strict: bool = False):
+    def __init__(self, left_file: str, right_file: str, l_sample_id_key: str, r_sample_id_key: str = None, config: str = None, strict: bool = False):
         """Initialise a file comparison object with reference and query datasets
 
         Args:
             left_data (dict): Data formatted by FileIO from the reference table
             right_data (dict, optional): Data formatted by FileIO from the query/subject table
             strict (bool, optional): Whether to elevate warnings to exceptions and terminate the program. Defaults to False.
         """
-        self.ref_table = left_data
-        self.query_table = right_data
+
+        self.ref_file = left_file
+        self.query_file = right_file
+
+        ## load reference data
+        ## if config provided then this will be used to map column names
+        self.ref_table = File_IO(input_file=left_file, sample_id_key=l_sample_id_key, config=config).data_dict
+
+        ## load query data
+        ## if unique key provided for sample_id column, use that
+        ## else assume same sampl_id_key as reference data
+        if r_sample_id_key:
+            self.query_table = File_IO(input_file=right_file, sample_id_key=r_sample_id_key).data_dict
+        else:
+            print("No sample_id_key provided for query table, assuming it is the same as sample_id_key for reference table.")
+            try:
+                self.query_table = File_IO(input_file=right_file, sample_id_key=l_sample_id_key).data_dict
+            except KeyError as ke:
+                raise ValueError(f"\n==\nDid not find any column {l_sample_id_key} in query table. Please check that you have provided a config file and a valid query sample_id_key on the command line.\n==\n")
+
         self.schema_errors = defaultdict(dict)
         self.value_mismatches = defaultdict(dict)
 
         ## if strict, ask warnings to be elevated to terminating exceptions
         if strict:
             warnings.filterwarnings("error")
 
-    def create_schema(self, required_fields: list = None):
+    def _create_schema(self, required_fields: list = None):
         """Using the reference data, infer a "schema": expected field names and expected data types for each field
 
         Args:
@@ -62,7 +81,7 @@ def create_schema(self, required_fields: list = None):
             for k in self.sample_schema.keys():
                 self.sample_schema[k].update({"required": True})
 
-    def confirm_schema(self):
+    def _confirm_schema(self):
         """Function to check data types in the query/subject table to ensure it matches the schema inferred from the reference table
         """
 
@@ -106,6 +125,13 @@ def compare_values(self, field_names: list = None, col_maps: dict = None):
             field_names (list, optional): List of field names to compare values for. Defaults to None and all fields are checked.
             col_maps (dict, optional): Mapping of column names between reference and query datasets if the schemas are not identical. Defaults to None.
         """
+
+        ## create schema from ref_table
+        ## check query_table schema against it
+        print("Setting up and validating schema...")
+        self._create_schema()
+        self._confirm_schema()
+
         if col_maps:
             raise NotImplementedError("Operation mode that uses this argument is not yet implemented.")