From 7f4f2c170b21ba424bc4937d61b0c65ac5d2aea0 Mon Sep 17 00:00:00 2001 From: linglp Date: Mon, 17 Jun 2024 15:10:28 -0400 Subject: [PATCH 1/4] black schematic-api --- .pre-commit-config.yaml | 2 +- schematic_api/api/__main__.py | 20 ++++++++++++++++++-- schematic_api/api/routes.py | 13 +++++++++---- schematic_api/api/security_controller_.py | 2 +- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 623446ced..e4b90e42e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,4 +10,4 @@ repos: # pre-commit's default_language_version, see # https://pre-commit.com/#top_level-default_language_version language_version: python3.10 - files: schematic/ \ No newline at end of file + files: ^(tests|schematic|schematic_api)/ \ No newline at end of file diff --git a/schematic_api/api/__main__.py b/schematic_api/api/__main__.py index 923cebaf5..316ce4e59 100644 --- a/schematic_api/api/__main__.py +++ b/schematic_api/api/__main__.py @@ -1,8 +1,23 @@ import os from schematic_api.api import app +import traceback +import jsonify -def main(): +@app.errorhandler(Exception) +def handle_exception(e): + # Get the last line of the traceback + last_line = traceback.format_exc().strip().split("\n")[-1] + + # Log the full traceback (optional) + app.logger.error(traceback.format_exc()) + + # Return a JSON response with the last line of the error + response = {"status": "error", "message": last_line} + return jsonify(response), 500 + + +def main(): # Get app configuration host = os.environ.get("APP_HOST", "0.0.0.0") port = os.environ.get("APP_PORT", "3001") @@ -11,5 +26,6 @@ def main(): # Launch app app.run(host=host, port=port, debug=False) + if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/schematic_api/api/routes.py b/schematic_api/api/routes.py index fbf36fbf5..9bc53ff5a 100644 --- a/schematic_api/api/routes.py +++ b/schematic_api/api/routes.py @@ -39,7 +39,10 @@ SynapseTimeoutError, ) from schematic.utils.general import entity_type_mapping -from schematic.utils.schema_utils import get_property_label_from_display_name, DisplayLabelType +from schematic.utils.schema_utils import ( + get_property_label_from_display_name, + DisplayLabelType, +) logger = logging.getLogger(__name__) logging.basicConfig(level=logging.DEBUG) @@ -210,6 +213,7 @@ def save_file(file_key="csv_file"): return temp_path + def initalize_metadata_model(schema_url, data_model_labels): # get path to temp data model file (csv or jsonld) as appropriate data_model = get_temp_model_path(schema_url) @@ -393,7 +397,7 @@ def submit_manifest_route( project_scope=None, table_column_names=None, annotation_keys=None, - file_annotations_upload:bool=True, + file_annotations_upload: bool = True, ): # call config_handler() config_handler(asset_view=asset_view) @@ -450,7 +454,7 @@ def submit_manifest_route( project_scope=project_scope, table_column_names=table_column_names, annotation_keys=annotation_keys, - file_annotations_upload=file_annotations_upload + file_annotations_upload=file_annotations_upload, ) return manifest_id @@ -729,6 +733,7 @@ def get_asset_view_table(asset_view, return_type): file_view_table_df.to_csv(export_path, index=False) return export_path + def get_project_manifests(project_id, asset_view): # Access token now stored in request header access_token = get_access_token() @@ -1022,4 +1027,4 @@ def get_schematic_version() -> str: raise NotImplementedError( "Using this endpoint to check the version of schematic is only supported when the API is running in a docker container." ) - return version \ No newline at end of file + return version diff --git a/schematic_api/api/security_controller_.py b/schematic_api/api/security_controller_.py index ee336dcb0..fbde596bb 100644 --- a/schematic_api/api/security_controller_.py +++ b/schematic_api/api/security_controller_.py @@ -11,4 +11,4 @@ def info_from_bearerAuth(token): :return: Decoded token information or None if token is invalid :rtype: dict | None """ - return {"uid": "user_id"} \ No newline at end of file + return {"uid": "user_id"} From 8218fbabc702cfa209c6d966241d5138047ed595 Mon Sep 17 00:00:00 2001 From: linglp Date: Mon, 17 Jun 2024 15:11:24 -0400 Subject: [PATCH 2/4] run black for all the tests --- tests/conftest.py | 11 +- tests/test_api.py | 21 +- tests/test_manifest.py | 129 +++++++--- tests/test_metadata.py | 6 +- tests/test_schemas.py | 13 +- tests/test_store.py | 19 +- tests/test_utils.py | 80 +++--- tests/test_validation.py | 536 ++++++++++++++++++++++++++------------- 8 files changed, 551 insertions(+), 264 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8d73650ef..62c2cb3e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -128,19 +128,22 @@ def synapse_store(request): # These fixtures make copies of existing test manifests. -# These copies can the be altered by a given test, and the copy will eb destroyed at the +# These copies can the be altered by a given test, and the copy will eb destroyed at the # end of the test + @pytest.fixture(scope="function") def temporary_file_copy(request, helpers: Helpers) -> Generator[str, None, None]: file_name = request.param # original file copy original_test_path = helpers.get_data_path(f"mock_manifests/{file_name}") # get filename without extension - file_name_no_extension=file_name.split(".")[0] + file_name_no_extension = file_name.split(".")[0] # Copy the original CSV file to a temporary directory - temp_csv_path = helpers.get_data_path(f"mock_manifests/{file_name_no_extension}_copy.csv") - + temp_csv_path = helpers.get_data_path( + f"mock_manifests/{file_name_no_extension}_copy.csv" + ) + shutil.copyfile(original_test_path, temp_csv_path) yield temp_csv_path # Teardown diff --git a/tests/test_api.py b/tests/test_api.py index 15c6786e7..97183186f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -78,7 +78,7 @@ def test_manifest_json(helpers): @pytest.fixture(scope="class") def data_model_jsonld(): - data_model_jsonld ="https://raw.githubusercontent.com/Sage-Bionetworks/schematic/develop/tests/data/example.model.jsonld" + data_model_jsonld = "https://raw.githubusercontent.com/Sage-Bionetworks/schematic/develop/tests/data/example.model.jsonld" yield data_model_jsonld @@ -143,7 +143,7 @@ class TestSynapseStorage: def test_invalid_authentication(self, client, request_invalid_headers): response = client.get( "http://localhost:3001/v1/storage/assets/tables", - query_string = {"asset_view": "syn23643253", "return_type": "csv"}, + query_string={"asset_view": "syn23643253", "return_type": "csv"}, headers=request_invalid_headers, ) assert response.status_code == 401 @@ -151,7 +151,7 @@ def test_invalid_authentication(self, client, request_invalid_headers): def test_insufficent_auth(self, client, request_headers): response = client.get( "http://localhost:3001/v1/storage/assets/tables", - query_string = {"asset_view": "syn23643252", "return_type": "csv"}, + query_string={"asset_view": "syn23643252", "return_type": "csv"}, headers=request_headers, ) assert response.status_code == 403 @@ -370,8 +370,7 @@ def test_get_property_label_from_display_name(self, client, strict_camel_case): @pytest.mark.schematic_api class TestDataModelGraphExplorerOperation: def test_get_schema(self, client, data_model_jsonld): - params = {"schema_url": data_model_jsonld, - "data_model_labels": 'class_label'} + params = {"schema_url": data_model_jsonld, "data_model_labels": "class_label"} response = client.get( "http://localhost:3001/v1/schemas/get/schema", query_string=params ) @@ -385,7 +384,11 @@ def test_get_schema(self, client, data_model_jsonld): os.remove(response_dt) def test_if_node_required(test, client, data_model_jsonld): - params = {"schema_url": data_model_jsonld, "node_display_name": "FamilyHistory", "data_model_labels": "class_label"} + params = { + "schema_url": data_model_jsonld, + "node_display_name": "FamilyHistory", + "data_model_labels": "class_label", + } response = client.get( "http://localhost:3001/v1/schemas/is_node_required", query_string=params @@ -1121,7 +1124,11 @@ def test_submit_manifest_file_only_replace( elif python_version == "3.9": dataset_id = "syn52656104" - specific_params = {"asset_view": "syn23643253", "dataset_id": dataset_id, "project_scope":["syn54126707"]} + specific_params = { + "asset_view": "syn23643253", + "dataset_id": dataset_id, + "project_scope": ["syn54126707"], + } params.update(specific_params) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 0525b6c6a..da88dda95 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -61,7 +61,9 @@ def manifest_generator(helpers, request): # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) manifest_generator = ManifestGenerator( @@ -111,18 +113,22 @@ def manifest(dataset_id, manifest_generator, request): yield manifest, use_annotations, data_type, sheet_url + @pytest.fixture(scope="class") def app(): app = create_app() yield app + class TestManifestGenerator: def test_init(self, helpers): path_to_data_model = helpers.get_data_path("example.model.jsonld") # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) generator = ManifestGenerator( @@ -157,7 +163,9 @@ def test_missing_root_error(self, helpers, data_type, exc, exc_message): # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) # A LookupError should be raised and include message when the component cannot be found @@ -242,7 +250,9 @@ def test_get_manifest_excel(self, helpers, sheet_url, output_format, dataset_id) # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) generator = ManifestGenerator( @@ -300,7 +310,9 @@ def test_get_manifest_no_annos(self, helpers, dataset_id): # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) # Instantiate object with use_annotations set to True @@ -416,7 +428,9 @@ def test_add_root_to_component_without_additional_metadata( # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) manifest_generator = ManifestGenerator( @@ -453,7 +467,9 @@ def test_add_root_to_component_with_additional_metadata( # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) manifest_generator = ManifestGenerator( @@ -537,7 +553,9 @@ def test_update_dataframe_with_existing_df(self, helpers, existing_manifest): # Get graph data model graph_data_model = generate_graph_data_model( - helpers, path_to_data_model=path_to_data_model, data_model_labels='class_label', + helpers, + path_to_data_model=path_to_data_model, + data_model_labels="class_label", ) # Instantiate the Manifest Generator. @@ -661,34 +679,85 @@ def test_populate_existing_excel_spreadsheet( # remove file os.remove(dummy_output_path) - - @pytest.mark.parametrize("return_output", ["Mock excel file path", "Mock google sheet link"]) - def test_create_single_manifest(self, simple_manifest_generator, helpers, return_output): - with patch("schematic.manifest.generator.ManifestGenerator.get_manifest", return_value=return_output): + + @pytest.mark.parametrize( + "return_output", ["Mock excel file path", "Mock google sheet link"] + ) + def test_create_single_manifest( + self, simple_manifest_generator, helpers, return_output + ): + with patch( + "schematic.manifest.generator.ManifestGenerator.get_manifest", + return_value=return_output, + ): json_ld_path = helpers.get_data_path("example.model.jsonld") data_type = "Patient" - graph_data_model = generate_graph_data_model(helpers, path_to_data_model=json_ld_path, data_model_labels='class_label') + graph_data_model = generate_graph_data_model( + helpers, + path_to_data_model=json_ld_path, + data_model_labels="class_label", + ) - result = simple_manifest_generator.create_single_manifest(path_to_data_model=json_ld_path, graph_data_model=graph_data_model, data_type=data_type, output_format="google_sheet", use_annotations=False) + result = simple_manifest_generator.create_single_manifest( + path_to_data_model=json_ld_path, + graph_data_model=graph_data_model, + data_type=data_type, + output_format="google_sheet", + use_annotations=False, + ) assert result == return_output - - @pytest.mark.parametrize("test_data_types", [["Patient", "Biospecimen"], ["all manifests"]]) - def test_create_manifests_raise_errors(self, simple_manifest_generator, helpers, test_data_types): - with pytest.raises(ValueError) as exception_info: + + @pytest.mark.parametrize( + "test_data_types", [["Patient", "Biospecimen"], ["all manifests"]] + ) + def test_create_manifests_raise_errors( + self, simple_manifest_generator, helpers, test_data_types + ): + with pytest.raises(ValueError) as exception_info: json_ld_path = helpers.get_data_path("example.model.jsonld") data_types = test_data_types - dataset_ids=["syn123456"] - - simple_manifest_generator.create_manifests(path_to_data_model=json_ld_path, data_types=data_types, dataset_ids=dataset_ids, output_format="google_sheet", use_annotations=False, data_model_labels='class_label') - - @pytest.mark.parametrize("test_data_types, dataset_ids, expected_result", [ - (["Patient", "Biospecimen"], ["mock dataset id1", "mock dataset id2"], ["mock google sheet link", "mock google sheet link"]), - (["Patient"], ["mock dataset id1"], ["mock google sheet link"]), - ]) - def test_create_manifests(self, simple_manifest_generator, helpers, test_data_types, dataset_ids, expected_result): - with patch("schematic.manifest.generator.ManifestGenerator.create_single_manifest", return_value="mock google sheet link"): + dataset_ids = ["syn123456"] + + simple_manifest_generator.create_manifests( + path_to_data_model=json_ld_path, + data_types=data_types, + dataset_ids=dataset_ids, + output_format="google_sheet", + use_annotations=False, + data_model_labels="class_label", + ) + + @pytest.mark.parametrize( + "test_data_types, dataset_ids, expected_result", + [ + ( + ["Patient", "Biospecimen"], + ["mock dataset id1", "mock dataset id2"], + ["mock google sheet link", "mock google sheet link"], + ), + (["Patient"], ["mock dataset id1"], ["mock google sheet link"]), + ], + ) + def test_create_manifests( + self, + simple_manifest_generator, + helpers, + test_data_types, + dataset_ids, + expected_result, + ): + with patch( + "schematic.manifest.generator.ManifestGenerator.create_single_manifest", + return_value="mock google sheet link", + ): json_ld_path = helpers.get_data_path("example.model.jsonld") - all_results = simple_manifest_generator.create_manifests(path_to_data_model=json_ld_path, data_types=test_data_types, dataset_ids=dataset_ids, output_format="google_sheet", use_annotations=False, data_model_labels='class_label') + all_results = simple_manifest_generator.create_manifests( + path_to_data_model=json_ld_path, + data_types=test_data_types, + dataset_ids=dataset_ids, + output_format="google_sheet", + use_annotations=False, + data_model_labels="class_label", + ) assert all_results == expected_result - diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 8a2c2e965..bf0c4d97b 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -109,10 +109,12 @@ def test_populate_manifest(self, helpers, return_excel, data_model_labels): ids=["data_model_labels-display_label", "data_model_labels-class_label"], ) @pytest.mark.parametrize("validate_component", [None, "BulkRNA-seqAssay"]) - @pytest.mark.parametrize("temporary_file_copy", ["test_BulkRNAseq.csv"], indirect=True) + @pytest.mark.parametrize( + "temporary_file_copy", ["test_BulkRNAseq.csv"], indirect=True + ) def test_submit_metadata_manifest( self, - temporary_file_copy: Generator[str, None, None], + temporary_file_copy: Generator[str, None, None], helpers: Helpers, file_annotations_upload: bool, restrict_rules: bool, diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 61479a3e8..f80449b18 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -20,7 +20,7 @@ convert_bool_to_str, parse_validation_rules, DisplayLabelType, - get_json_schema_log_file_path + get_json_schema_log_file_path, ) from schematic.utils.io_utils import load_json @@ -448,9 +448,12 @@ def test_generate_data_model_graph(self, helpers, data_model, data_model_labels) # Check that all relationships recorded between 'CheckList' and 'Ab' are present assert ( - "rangeValue" and "parentOf" in graph["CheckListEnum"][expected_valid_values[0]] + "rangeValue" + and "parentOf" in graph["CheckListEnum"][expected_valid_values[0]] + ) + assert ( + "requiresDependency" not in graph["CheckListEnum"][expected_valid_values[0]] ) - assert "requiresDependency" not in graph["CheckListEnum"][expected_valid_values[0]] # Check nodes: assert "Patient" in graph.nodes @@ -1325,8 +1328,8 @@ def test_get_json_validation_schema( data_model_path = helpers.get_data_path(path=data_model) json_schema_log_file_path = get_json_schema_log_file_path( - data_model_path=data_model_path, - source_node=source_node) + data_model_path=data_model_path, source_node=source_node + ) # Remove json schema log file if it already exists. if os.path.exists(json_schema_log_file_path): diff --git a/tests/test_store.py b/tests/test_store.py index 06fa4bf23..7a162c6e1 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -22,11 +22,7 @@ from tests.conftest import Helpers from schematic.store.base import BaseStorage -from schematic.store.synapse import ( - DatasetFileView, - ManifestDownload, - SynapseStorage -) +from schematic.store.synapse import DatasetFileView, ManifestDownload, SynapseStorage from schematic.utils.general import check_synapse_cache_size logging.basicConfig(level=logging.DEBUG) @@ -131,7 +127,7 @@ def test_init(self): class TestSynapseStorage: "Tests the SynapseStorage class" - def test_init(self, synapse_store:SynapseStorage) -> None: + def test_init(self, synapse_store: SynapseStorage) -> None: """Tests SynapseStorage.__init__""" assert synapse_store.storageFileview == "syn23643253" assert isinstance(synapse_store.storageFileviewTable, pd.DataFrame) @@ -142,8 +138,7 @@ def test__purge_synapse_cache(self) -> None: synapse_store = SynapseStorage(synapse_cache_path="test_cache_dir") size_before_purge = check_synapse_cache_size(synapse_store.root_synapse_cache) synapse_store._purge_synapse_cache( - maximum_storage_allowed_cache_gb=0.000001, - minute_buffer=0 + maximum_storage_allowed_cache_gb=0.000001, minute_buffer=0 ) size_after_purge = check_synapse_cache_size(synapse_store.root_synapse_cache) assert size_before_purge > size_after_purge @@ -157,7 +152,7 @@ def test_login(self) -> None: assert synapse_client.cache.cache_root_dir == "test_cache_dir" shutil.rmtree("test_cache_dir") - def test_getFileAnnotations(self, synapse_store:SynapseStorage) -> None: + def test_getFileAnnotations(self, synapse_store: SynapseStorage) -> None: expected_dict = { "author": "bruno, milen, sujay", "impact": "42.9", @@ -220,17 +215,17 @@ def test_get_file_entityIds(self, helpers, synapse_store, only_new_files): {"CheckInt": "7", "CheckList": "valid, list, values"}, "syn34295552", "file_and_entities", - "annotations_test_manifest.csv" + "annotations_test_manifest.csv", ), ( {"FileFormat": "BAM", "GenomeBuild": "GRCh38"}, "syn39241199", "table_and_file", - "test_BulkRNAseq.csv" + "test_BulkRNAseq.csv", ), ], ids=["non file-based", "file-based"], - indirect=["temporary_file_copy"] + indirect=["temporary_file_copy"], ) def test_annotation_submission( self, diff --git a/tests/test_utils.py b/tests/test_utils.py index 1ff72d673..5b37abe6e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -11,6 +11,7 @@ from typing import Union, Generator from _pytest.fixtures import FixtureRequest + import numpy as np import pandas as pd import pytest @@ -196,7 +197,8 @@ (1073741825, 1073741824, 1181116006.4), ] -def get_metadataModel(helpers, model_name:str): + +def get_metadataModel(helpers, model_name: str): metadataModel = MetadataModel( inputMModelLocation=helpers.get_data_path(model_name), inputMModelLocationType="local", @@ -1025,26 +1027,27 @@ def test_get_label_from_display_name(self, test_dn: str, data_model_labels: str) return @pytest.mark.parametrize( - "data_model", - list(DATA_MODEL_DICT.keys()), - ids=list(DATA_MODEL_DICT.values()) + "data_model", list(DATA_MODEL_DICT.keys()), ids=list(DATA_MODEL_DICT.values()) ) @pytest.mark.parametrize( "source_node", ["Biospecimen", "Patient"], ids=["biospecimen_source", "patient_source"], ) - def test_get_json_schema_log_file_path(self, helpers, data_model:str, source_node: str): + def test_get_json_schema_log_file_path( + self, helpers, data_model: str, source_node: str + ): data_model_path = helpers.get_data_path(path=data_model) json_schema_log_file_path = get_json_schema_log_file_path( - data_model_path=data_model_path, - source_node=source_node) + data_model_path=data_model_path, source_node=source_node + ) # Check that model is not included in the json_schema_log_file_path - assert '.model' not in "data_model" + assert ".model" not in "data_model" # Check the file suffixs are what is expected. - assert ['schema', 'json'] == json_schema_log_file_path.split('.')[-2:] + assert ["schema", "json"] == json_schema_log_file_path.split(".")[-2:] + class TestValidateUtils: def test_validate_schema(self, helpers): @@ -1098,13 +1101,22 @@ def test_validate_property_schema(self, helpers): @pytest.mark.parametrize( ("manifest", "model", "root_node"), - [("mock_manifests/Patient_test_no_entry_for_cond_required_column.manifest.csv", - "example.model.csv", "Patient"), - ("mock_manifests/Valid_Test_Manifest_with_nones.csv", - "example_test_nones.model.csv", "MockComponent")] - ) + [ + ( + "mock_manifests/Patient_test_no_entry_for_cond_required_column.manifest.csv", + "example.model.csv", + "Patient", + ), + ( + "mock_manifests/Valid_Test_Manifest_with_nones.csv", + "example_test_nones.model.csv", + "MockComponent", + ), + ], + ) def test_convert_nan_entries_to_empty_strings( - self, helpers, manifest, model, root_node): + self, helpers, manifest, model, root_node + ): # Get manifest and data model path manifest_path = helpers.get_data_path(manifest) model_path = helpers.get_data_path(model) @@ -1128,37 +1140,37 @@ def test_convert_nan_entries_to_empty_strings( manifest_path, preserve_raw_input=False, allow_na_values=True, - **load_args,) + **load_args, + ) metadataModel = get_metadataModel(helpers, model) # Instantiate Validate manifest, and run manifest validation - # In this step the manifest is modified while running rule + # In this step the manifest is modified while running rule # validation so need to do this step to get the updated manfest. - vm = ValidateManifest( - errors, manifest, manifest_path, dmge, json_schema) + vm = ValidateManifest(errors, manifest, manifest_path, dmge, json_schema) manifest, vmr_errors, vmr_warnings = vm.validate_manifest_rules( - manifest, dmge, restrict_rules=False, project_scope=["syn54126707"], + manifest, + dmge, + restrict_rules=False, + project_scope=["syn54126707"], ) # Run convert nan function - output = validate_utils.convert_nan_entries_to_empty_strings( - manifest=manifest - ) + output = validate_utils.convert_nan_entries_to_empty_strings(manifest=manifest) # Compare post rule validation manifest with output manifest looking # for expected nan to empty string conversion - if root_node == 'Patient': - assert manifest['Family History'][0] == [''] - assert output['Family History'][0] == [''] - elif root_node == 'MockComponent': - assert manifest['Check List'][2] == [''] - assert manifest['Check List Like Enum'][2] == [] - assert type(manifest['Check NA'][2]) == type(pd.NA) - - assert output['Check List'][2] == [''] - assert output['Check List Like Enum'][2] == [] - + if root_node == "Patient": + assert manifest["Family History"][0] == [""] + assert output["Family History"][0] == [""] + elif root_node == "MockComponent": + assert manifest["Check List"][2] == [""] + assert manifest["Check List Like Enum"][2] == [] + assert type(manifest["Check NA"][2]) == type(pd.NA) + + assert output["Check List"][2] == [""] + assert output["Check List Like Enum"][2] == [] def test_get_list_robustness(self, helpers): return diff --git a/tests/test_validation.py b/tests/test_validation.py index b2b85851d..9ea47b973 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -27,7 +27,8 @@ def DMGE(helpers): dmge = helpers.get_data_model_graph_explorer(path="example.model.jsonld") yield dmge -def get_metadataModel(helpers, model_name:str): + +def get_metadataModel(helpers, model_name: str): metadataModel = MetadataModel( inputMModelLocation=helpers.get_data_path(model_name), inputMModelLocationType="local", @@ -55,20 +56,47 @@ class TestManifestValidation: @pytest.mark.parametrize( ("model_name", "manifest_name", "root_node"), [ - ("example.model.csv","mock_manifests/Valid_Test_Manifest.csv", "MockComponent"), - ("example.model.csv", "mock_manifests/Patient_test_no_entry_for_cond_required_column.manifest.csv", "Patient"), - ("example_test_nones.model.csv","mock_manifests/Valid_Test_Manifest_with_nones.csv", "MockComponent"), + ( + "example.model.csv", + "mock_manifests/Valid_Test_Manifest.csv", + "MockComponent", + ), + ( + "example.model.csv", + "mock_manifests/Patient_test_no_entry_for_cond_required_column.manifest.csv", + "Patient", + ), + ( + "example_test_nones.model.csv", + "mock_manifests/Valid_Test_Manifest_with_nones.csv", + "MockComponent", + ), + ], + ids=[ + "example_model", + "example_with_no_entry_for_cond_required_columns", + "example_with_nones", ], - ids=["example_model", "example_with_no_entry_for_cond_required_columns", "example_with_nones"], ) @pytest.mark.parametrize( "project_scope", ["syn54126707", "syn55250368", "syn55271234"], - ids=["project_scope_with_manifests", "project_scope_without_manifests", "project_scope_with_empty_manifest"], + ids=[ + "project_scope_with_manifests", + "project_scope_without_manifests", + "project_scope_with_empty_manifest", + ], ) - def test_valid_manifest(self, helpers, model_name:str, manifest_name:str, - root_node:str, project_scope:str, dmge:DataModelGraph): - """ Run the valid manifest in various situations, some of which will generate errors or warnings, + def test_valid_manifest( + self, + helpers, + model_name: str, + manifest_name: str, + root_node: str, + project_scope: str, + dmge: DataModelGraph, + ): + """Run the valid manifest in various situations, some of which will generate errors or warnings, if there are "issues" with target manifests on manifests. Since there are so many parameters, limit the combinations that are being run to the ones that are relevant. Args: @@ -90,16 +118,28 @@ def test_valid_manifest(self, helpers, model_name:str, manifest_name:str, manifest_path = helpers.get_data_path(manifest_name) warning_rule_sets_1 = [ - ('Check Match at Least', 'matchAtLeastOne Patient.PatientID set'), - ('Check Match at Least values', 'matchAtLeastOne MockComponent.checkMatchatLeastvalues value'), - ('Check Match Exactly', 'matchExactlyOne MockComponent.checkMatchExactly set'), - ('Check Match Exactly values', 'matchExactlyOne MockComponent.checkMatchExactlyvalues value'), - ] + ("Check Match at Least", "matchAtLeastOne Patient.PatientID set"), + ( + "Check Match at Least values", + "matchAtLeastOne MockComponent.checkMatchatLeastvalues value", + ), + ( + "Check Match Exactly", + "matchExactlyOne MockComponent.checkMatchExactly set", + ), + ( + "Check Match Exactly values", + "matchExactlyOne MockComponent.checkMatchExactlyvalues value", + ), + ] warning_rule_sets_2 = warning_rule_sets_1[1:] error_rule_sets = [ - ('Check Match None', 'matchNone MockComponent.checkMatchNone set error'), - ('Check Match None values', 'matchNone MockComponent.checkMatchNonevalues value error'), - ] + ("Check Match None", "matchNone MockComponent.checkMatchNone set error"), + ( + "Check Match None values", + "matchNone MockComponent.checkMatchNonevalues value error", + ), + ] # For the standard project scope, models and manifest should pass without warnings or errors if project_scope == "syn54126707": @@ -113,25 +153,34 @@ def test_valid_manifest(self, helpers, model_name:str, manifest_name:str, # When submitting the first manifest for cross manifest validation (MockComponent), check that proper warning # (to alert users that no validation will be run), is raised. The manifest is still valid to submit. - if (project_scope == "syn55250368" and root_node=="MockComponent" and - model_name in ["example.model.csv", "example_test_nones.model.csv"]): + if ( + project_scope == "syn55250368" + and root_node == "MockComponent" + and model_name in ["example.model.csv", "example_test_nones.model.csv"] + ): metadataModel = get_metadataModel(helpers, model_name) errors, warnings = metadataModel.validateModelManifest( manifestPath=manifest_path, rootNode=root_node, project_scope=[project_scope], ) - + for attribute_name, val_rule in warning_rule_sets_1: - assert GenerateError.generate_no_cross_warning( - dmge=dmge, - attribute_name=attribute_name, - val_rule=val_rule)[0] in warnings + assert ( + GenerateError.generate_no_cross_warning( + dmge=dmge, attribute_name=attribute_name, val_rule=val_rule + )[0] + in warnings + ) assert errors == [] - + # When submitting a manifest to a project that contains a manifest without data, ensure that the proper # warnings/errors are raised. - elif project_scope == "syn55271234" and root_node=="MockComponent" and model_name == "example.model.csv": + elif ( + project_scope == "syn55271234" + and root_node == "MockComponent" + and model_name == "example.model.csv" + ): metadataModel = get_metadataModel(helpers, model_name) errors, warnings = metadataModel.validateModelManifest( manifestPath=manifest_path, @@ -139,21 +188,24 @@ def test_valid_manifest(self, helpers, model_name:str, manifest_name:str, project_scope=[project_scope], ) for attribute_name, val_rule in warning_rule_sets_2: - assert GenerateError.generate_no_value_in_manifest_error( - dmge=dmge, - attribute_name=attribute_name, - val_rule=val_rule)[1][0] in warnings - - for attribute_name, val_rule in error_rule_sets: - assert GenerateError.generate_no_value_in_manifest_error( - dmge=dmge, - attribute_name=attribute_name, - val_rule=val_rule)[0][0] in errors + assert ( + GenerateError.generate_no_value_in_manifest_error( + dmge=dmge, attribute_name=attribute_name, val_rule=val_rule + )[1][0] + in warnings + ) + for attribute_name, val_rule in error_rule_sets: + assert ( + GenerateError.generate_no_value_in_manifest_error( + dmge=dmge, attribute_name=attribute_name, val_rule=val_rule + )[0][0] + in errors + ) def test_invalid_manifest(self, helpers, dmge): metadataModel = get_metadataModel(helpers, model_name="example.model.jsonld") - + manifestPath = helpers.get_data_path("mock_manifests/Invalid_Test_Manifest.csv") rootNode = "MockComponent" @@ -164,31 +216,41 @@ def test_invalid_manifest(self, helpers, dmge): ) # Check errors - assert GenerateError.generate_type_error( + assert ( + GenerateError.generate_type_error( val_rule="num", row_num="3", attribute_name="Check Num", invalid_entry="c", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_type_error( + assert ( + GenerateError.generate_type_error( val_rule="int", row_num="3", attribute_name="Check Int", invalid_entry="5.63", dmge=dmge, - )[0] in errors - - assert GenerateError.generate_type_error( + )[0] + in errors + ) + + assert ( + GenerateError.generate_type_error( val_rule="str", row_num="3", attribute_name="Check String", invalid_entry="94", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="9", row_num="3", @@ -196,9 +258,12 @@ def test_invalid_manifest(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="9", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="ab", row_num="4", @@ -206,9 +271,12 @@ def test_invalid_manifest(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="ab", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="a c f", row_num="3", @@ -216,9 +284,12 @@ def test_invalid_manifest(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="a c f", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="a", row_num="4", @@ -226,9 +297,12 @@ def test_invalid_manifest(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="a", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="a", row_num="4", @@ -236,9 +310,12 @@ def test_invalid_manifest(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="a", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_regex_error( + assert ( + GenerateError.generate_regex_error( val_rule="regex", reg_expression="[a-f]", row_num="3", @@ -246,9 +323,12 @@ def test_invalid_manifest(self, helpers, dmge): module_to_call="match", invalid_entry="m", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_regex_error( + assert ( + GenerateError.generate_regex_error( val_rule="regex", reg_expression="[a-f]", row_num="3", @@ -256,9 +336,12 @@ def test_invalid_manifest(self, helpers, dmge): module_to_call="search", invalid_entry="q", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_regex_error( + assert ( + GenerateError.generate_regex_error( val_rule="regex", reg_expression="^\d+$", row_num="2", @@ -266,9 +349,12 @@ def test_invalid_manifest(self, helpers, dmge): module_to_call="search", invalid_entry="5.4", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_url_error( + assert ( + GenerateError.generate_url_error( val_rule="url", url="http://googlef.com/", url_error="invalid_url", @@ -277,7 +363,9 @@ def test_invalid_manifest(self, helpers, dmge): argument=None, invalid_entry="http://googlef.com/", dmge=dmge, - )[0] in errors + )[0] + in errors + ) date_err = GenerateError.generate_content_error( val_rule="date", @@ -289,21 +377,27 @@ def test_invalid_manifest(self, helpers, dmge): error_in_list = [date_err[2] in error for error in errors] assert any(error_in_list) - assert GenerateError.generate_content_error( + assert ( + GenerateError.generate_content_error( val_rule="unique error", attribute_name="Check Unique", dmge=dmge, row_num=["2", "3", "4"], invalid_entry=["str1"], - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_content_error( + assert ( + GenerateError.generate_content_error( val_rule="inRange 50 100 error", attribute_name="Check Range", dmge=dmge, row_num=["3"], invalid_entry=["30"], - )[0] in errors + )[0] + in errors + ) assert ( GenerateError.generate_cross_warning( @@ -314,7 +408,7 @@ def test_invalid_manifest(self, helpers, dmge): invalid_entry=["123"], dmge=dmge, )[0] - in errors + in errors ) assert ( @@ -325,54 +419,69 @@ def test_invalid_manifest(self, helpers, dmge): invalid_entry=["123"], dmge=dmge, )[0] - in errors + in errors ) # check warnings - assert GenerateError.generate_content_error( + assert ( + GenerateError.generate_content_error( val_rule="recommended", attribute_name="Check Recommended", dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) - assert GenerateError.generate_content_error( + assert ( + GenerateError.generate_content_error( val_rule="protectAges", attribute_name="Check Ages", dmge=dmge, row_num=["2", "3"], invalid_entry=["6549", "32851"], - )[1] in warnings + )[1] + in warnings + ) - assert GenerateError.generate_cross_warning( + assert ( + GenerateError.generate_cross_warning( val_rule="matchAtLeastOne", row_num=["3"], attribute_name="Check Match at Least", invalid_entry=["7163"], manifest_id=["syn54126997", "syn54127001"], dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) - assert GenerateError.generate_cross_warning( + assert ( + GenerateError.generate_cross_warning( val_rule="matchAtLeastOne MockComponent.checkMatchatLeastvalues value", row_num=["3"], attribute_name="Check Match at Least values", invalid_entry=["51100"], dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) - assert \ + assert ( GenerateError.generate_cross_warning( val_rule="matchExactlyOne", attribute_name="Check Match Exactly", matching_manifests=["syn54126950", "syn54127008"], dmge=dmge, - )[1] in warnings \ + )[1] + in warnings or GenerateError.generate_cross_warning( val_rule="matchExactlyOne", attribute_name="Check Match Exactly", matching_manifests=["syn54127702", "syn54127008"], dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) cross_warning = GenerateError.generate_cross_warning( val_rule="matchExactlyOne MockComponent.checkMatchExactlyvalues MockComponent.checkMatchExactlyvalues value", @@ -385,7 +494,6 @@ def test_invalid_manifest(self, helpers, dmge): warning_in_list = [cross_warning[1] in warning for warning in warnings] assert any(warning_in_list) - def test_in_house_validation(self, helpers, dmge): metadataModel = get_metadataModel(helpers, model_name="example.model.jsonld") manifestPath = helpers.get_data_path("mock_manifests/Invalid_Test_Manifest.csv") @@ -399,39 +507,52 @@ def test_in_house_validation(self, helpers, dmge): ) # Check errors - assert GenerateError.generate_type_error( + assert ( + GenerateError.generate_type_error( val_rule="num", row_num="3", attribute_name="Check Num", invalid_entry="c", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_type_error( + assert ( + GenerateError.generate_type_error( val_rule="int", row_num="3", attribute_name="Check Int", invalid_entry="5.63", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_type_error( + assert ( + GenerateError.generate_type_error( val_rule="str", row_num="3", attribute_name="Check String", invalid_entry="94", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_type_error( + assert ( + GenerateError.generate_type_error( val_rule="int", row_num="3", attribute_name="Check NA", invalid_entry="9.5", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="9", row_num="3", @@ -439,9 +560,12 @@ def test_in_house_validation(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="9", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_list_error( + assert ( + GenerateError.generate_list_error( val_rule="list", list_string="ab", row_num="4", @@ -449,9 +573,12 @@ def test_in_house_validation(self, helpers, dmge): list_error="not_comma_delimited", invalid_entry="ab", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_regex_error( + assert ( + GenerateError.generate_regex_error( val_rule="regex", reg_expression="[a-f]", row_num="3", @@ -459,9 +586,12 @@ def test_in_house_validation(self, helpers, dmge): module_to_call="search", invalid_entry="q", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_regex_error( + assert ( + GenerateError.generate_regex_error( val_rule="regex", reg_expression="[a-f]", row_num="3", @@ -469,9 +599,12 @@ def test_in_house_validation(self, helpers, dmge): module_to_call="match", invalid_entry="m", dmge=dmge, - )[0] in errors + )[0] + in errors + ) - assert GenerateError.generate_url_error( + assert ( + GenerateError.generate_url_error( val_rule="url", url="http://googlef.com/", url_error="invalid_url", @@ -480,7 +613,9 @@ def test_in_house_validation(self, helpers, dmge): argument=None, invalid_entry="http://googlef.com/", dmge=dmge, - )[0] in errors + )[0] + in errors + ) assert ( GenerateError.generate_cross_warning( @@ -491,7 +626,7 @@ def test_in_house_validation(self, helpers, dmge): invalid_entry=["123"], dmge=dmge, )[0] - in errors + in errors ) assert ( @@ -502,56 +637,66 @@ def test_in_house_validation(self, helpers, dmge): invalid_entry=["123"], dmge=dmge, )[0] - in errors + in errors ) # Check Warnings - assert GenerateError.generate_cross_warning( + assert ( + GenerateError.generate_cross_warning( val_rule="matchAtLeastOne", row_num=["3"], attribute_name="Check Match at Least", invalid_entry=["7163"], manifest_id=["syn54126997", "syn54127001"], dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) - assert GenerateError.generate_cross_warning( + assert ( + GenerateError.generate_cross_warning( val_rule="matchAtLeastOne MockComponent.checkMatchatLeastvalues value", row_num=["3"], attribute_name="Check Match at Least values", invalid_entry=["51100"], dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) - assert \ + assert ( GenerateError.generate_cross_warning( val_rule="matchExactlyOne", attribute_name="Check Match Exactly", matching_manifests=["syn54126950", "syn54127008"], dmge=dmge, - )[1] in warnings \ + )[1] + in warnings or GenerateError.generate_cross_warning( val_rule="matchExactlyOne", attribute_name="Check Match Exactly", matching_manifests=["syn54127702", "syn54127008"], dmge=dmge, - )[1] in warnings + )[1] + in warnings + ) - assert GenerateError.generate_cross_warning( + assert ( + GenerateError.generate_cross_warning( val_rule="matchExactlyOne MockComponent.checkMatchExactlyvalues MockComponent.checkMatchExactlyvalues value", row_num=["2", "3", "4"], attribute_name="Check Match Exactly values", invalid_entry=["71738", "98085", "210065"], dmge=dmge, - )[1] in warnings - + )[1] + in warnings + ) - def test_missing_column(self, helpers, dmge:DataModelGraph): - """ Test that a manifest missing a column returns the proper error. - """ - model_name="example.model.csv" - manifest_name="mock_manifests/Invalid_Biospecimen_Missing_Column_Manifest.csv" - root_node="Biospecimen" + def test_missing_column(self, helpers, dmge: DataModelGraph): + """Test that a manifest missing a column returns the proper error.""" + model_name = "example.model.csv" + manifest_name = "mock_manifests/Invalid_Biospecimen_Missing_Column_Manifest.csv" + root_node = "Biospecimen" manifest_path = helpers.get_data_path(manifest_name) metadataModel = get_metadataModel(helpers, model_name) @@ -560,14 +705,16 @@ def test_missing_column(self, helpers, dmge:DataModelGraph): rootNode=root_node, ) - assert GenerateError.generate_schema_error( - row_num='2', + assert ( + GenerateError.generate_schema_error( + row_num="2", attribute_name="Wrong schema", error_message="'Tissue Status' is a required property", invalid_entry="Wrong schema", dmge=dmge, - )[0] in errors - + )[0] + in errors + ) @pytest.mark.parametrize( "model_name", @@ -577,19 +724,46 @@ def test_missing_column(self, helpers, dmge:DataModelGraph): ], ids=["example_model", "example_with_requirements_from_vr"], ) - @pytest.mark.parametrize( - ["manifest_name", "root_node",], [ - ("mock_manifests/Biospecimen_required_vr_test_fail.manifest.csv", "Biospecimen"), - ("mock_manifests/Biospecimen_required_vr_test_pass.manifest.csv", "Biospecimen"), + "manifest_name", + "root_node", + ], + [ + ( + "mock_manifests/Biospecimen_required_vr_test_fail.manifest.csv", + "Biospecimen", + ), + ( + "mock_manifests/Biospecimen_required_vr_test_pass.manifest.csv", + "Biospecimen", + ), ("mock_manifests/Patient_required_vr_test_pass.manifest.csv", "Patient"), - ("mock_manifests/Patient_test_no_entry_for_cond_required_column.manifest.csv", "Patient"), - ("mock_manifests/BulkRNAseq_component_based_required_rule_test.manifest.csv", "BulkRNA-seqAssay"), + ( + "mock_manifests/Patient_test_no_entry_for_cond_required_column.manifest.csv", + "Patient", + ), + ( + "mock_manifests/BulkRNAseq_component_based_required_rule_test.manifest.csv", + "BulkRNA-seqAssay", + ), + ], + ids=[ + "biospeciment_required_vr_empty", + "biospecimen_required_filled", + "patient_not_required_empty", + "patient_conditionally_required_not_filled", + "bulk_rna_seq_component_based_rule_test", ], - ids=["biospeciment_required_vr_empty", "biospecimen_required_filled", "patient_not_required_empty", "patient_conditionally_required_not_filled", "bulk_rna_seq_component_based_rule_test"], ) - def test_required_validation_rule(self, helpers, model_name:str, manifest_name:str, root_node:str, dmge:DataModelGraphExplorer) -> None: + def test_required_validation_rule( + self, + helpers, + model_name: str, + manifest_name: str, + root_node: str, + dmge: DataModelGraphExplorer, + ) -> None: """ Args: model_name, str: model to run test validation against @@ -630,7 +804,11 @@ def test_required_validation_rule(self, helpers, model_name:str, manifest_name:s rootNode=root_node, ) - error_and_warning_free_manifests = ["Biospecimen_required_vr_test_pass", "Patient_test_no_entry_for_cond_required_column", ""] + error_and_warning_free_manifests = [ + "Biospecimen_required_vr_test_pass", + "Patient_test_no_entry_for_cond_required_column", + "", + ] # For each model, these manifest should pass, bc either the value is being passed as requierd, or its not currently required for manifest in error_and_warning_free_manifests: @@ -638,70 +816,85 @@ def test_required_validation_rule(self, helpers, model_name:str, manifest_name:s assert errors == [] assert warnings == [] - messages = {"patient_id_empty_warning": { - "row_num":"2", - "attribute_name":"Patient ID", - "error_message":"'' should be non-empty", - "invalid_entry":""}, - "bulk_rnaseq_cbr_error_1":{ - "row_num":"3", - "attribute_name":"Genome FASTA", - "error_message":"'' should be non-empty", - "invalid_entry":""}, - "bulk_rnaseq_cbr_error_2":{ - "row_num":"4", - "attribute_name":"File Format", - "error_message":"'' is not one of ['CSV/TSV', 'CRAM', 'FASTQ', 'BAM']", - "invalid_entry":""}, - } + messages = { + "patient_id_empty_warning": { + "row_num": "2", + "attribute_name": "Patient ID", + "error_message": "'' should be non-empty", + "invalid_entry": "", + }, + "bulk_rnaseq_cbr_error_1": { + "row_num": "3", + "attribute_name": "Genome FASTA", + "error_message": "'' should be non-empty", + "invalid_entry": "", + }, + "bulk_rnaseq_cbr_error_2": { + "row_num": "4", + "attribute_name": "File Format", + "error_message": "'' is not one of ['CSV/TSV', 'CRAM', 'FASTQ', 'BAM']", + "invalid_entry": "", + }, + } # This manifest should fail in the example_model bc the manifest Required=False, and in the example_with_requirements_from_vr # bc the requirments are set to false in the validation rule - if (("Biospecimen_required_vr_test_fail" in manifest_name) or - ("Patient_required_vr_test_pass" in manifest_name and model_name == "example.model.csv") - ): + if ("Biospecimen_required_vr_test_fail" in manifest_name) or ( + "Patient_required_vr_test_pass" in manifest_name + and model_name == "example.model.csv" + ): message_key = "patient_id_empty_warning" - assert GenerateError.generate_schema_error( - row_num=messages[message_key]["row_num"], - attribute_name=messages[message_key]["attribute_name"], - error_message=messages[message_key]["error_message"], - invalid_entry=messages[message_key]["invalid_entry"], - dmge=dmge, - )[0] in errors + assert ( + GenerateError.generate_schema_error( + row_num=messages[message_key]["row_num"], + attribute_name=messages[message_key]["attribute_name"], + error_message=messages[message_key]["error_message"], + invalid_entry=messages[message_key]["invalid_entry"], + dmge=dmge, + )[0] + in errors + ) assert warnings == [] - if "Patient_required_vr_test_pass" in manifest_name and model_name == "example_required_vr_test.model.csv": + if ( + "Patient_required_vr_test_pass" in manifest_name + and model_name == "example_required_vr_test.model.csv" + ): assert errors == [] assert warnings == [] if "BulkRNAseq_component_based_required_rule_test" in manifest_name: message_key = "bulk_rnaseq_cbr_error_1" - assert GenerateError.generate_schema_error( + assert ( + GenerateError.generate_schema_error( row_num=messages[message_key]["row_num"], attribute_name=messages[message_key]["attribute_name"], error_message=messages[message_key]["error_message"], invalid_entry=messages[message_key]["invalid_entry"], dmge=dmge, - )[0] in errors + )[0] + in errors + ) message_key = "bulk_rnaseq_cbr_error_2" expected_error = GenerateError.generate_schema_error( - row_num=messages[message_key]["row_num"], - attribute_name=messages[message_key]["attribute_name"], - error_message=messages[message_key]["error_message"], - invalid_entry=messages[message_key]["invalid_entry"], - dmge=dmge, - )[0] + row_num=messages[message_key]["row_num"], + attribute_name=messages[message_key]["attribute_name"], + error_message=messages[message_key]["error_message"], + invalid_entry=messages[message_key]["invalid_entry"], + dmge=dmge, + )[0] # since the valid value order isnt set in error reporting, check a portion of the expected output # Check the error row is expected assert expected_error[1] in errors[1] # Check that one of the values for the expected valid values is present # Extract a valid value - valid_value = expected_error[2].split(',')[-1].split(']')[0].strip(' ').strip("\'") - assert valid_value in errors[1][2] - assert warnings==[] - + valid_value = ( + expected_error[2].split(",")[-1].split("]")[0].strip(" ").strip("'") + ) + assert valid_value in errors[1][2] + assert warnings == [] @pytest.mark.parametrize( "manifest_path", @@ -756,13 +949,16 @@ def test_component_validations(self, helpers, manifest_path, dmge): and vmr_warnings[0][-1] == ["123"] ) - @pytest.mark.rule_combos( reason="This introduces a great number of tests covering every possible rule combination that are only necessary on occasion." ) @pytest.mark.parametrize("base_rule, second_rule", get_rule_combinations()) def test_rule_combinations( - self, helpers, dmge, base_rule, second_rule, + self, + helpers, + dmge, + base_rule, + second_rule, ): """ TODO: Describe what this test is doing. From f9543fcca5d4a1d0595b1fd9ff141de2599b67d5 Mon Sep 17 00:00:00 2001 From: linglp Date: Mon, 17 Jun 2024 15:22:45 -0400 Subject: [PATCH 3/4] remove unintended code --- schematic_api/api/__main__.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/schematic_api/api/__main__.py b/schematic_api/api/__main__.py index 316ce4e59..afc24b44a 100644 --- a/schematic_api/api/__main__.py +++ b/schematic_api/api/__main__.py @@ -1,20 +1,5 @@ import os from schematic_api.api import app -import traceback -import jsonify - - -@app.errorhandler(Exception) -def handle_exception(e): - # Get the last line of the traceback - last_line = traceback.format_exc().strip().split("\n")[-1] - - # Log the full traceback (optional) - app.logger.error(traceback.format_exc()) - - # Return a JSON response with the last line of the error - response = {"status": "error", "message": last_line} - return jsonify(response), 500 def main(): From 0ea1aec431a35dc04ca77f8c797d1ce68de32392 Mon Sep 17 00:00:00 2001 From: linglp Date: Tue, 18 Jun 2024 13:57:16 -0400 Subject: [PATCH 4/4] update gh workflow --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b9e719dda..0b1a152ef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -90,7 +90,7 @@ jobs: run: | # ran only on certain files for now # add here when checked - poetry run black schematic --check + poetry run black schematic tests schematic_api --check #---------------------------------------------- # type checking/enforcement