From 0b35f3dfc17aba9465e0ac4a0feb94acfaee8ac1 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 13:45:16 -0700 Subject: [PATCH 1/9] changing TODO comment with `pytest.skip` --- tests/api/test_api.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/api/test_api.py b/tests/api/test_api.py index e460bfe08..858245eeb 100644 --- a/tests/api/test_api.py +++ b/tests/api/test_api.py @@ -41,11 +41,10 @@ def test_api_with_invalid_host() -> None: cript.API(host="no_http_host.org", api_token="123456789", storage_token="987654321") -# TODO commented out for now because it needs an API container +@pytest.mark.skip(reason="skipping for now because it needs an API container") def test_api_context(cript_api: cript.API) -> None: - # assert cript.api.api._global_cached_api is not None - # assert cript.api.api._get_global_cached_api() is not None - pass + assert cript.api.api._global_cached_api is not None + assert cript.api.api._get_global_cached_api() is not None def test_api_cript_env_vars() -> None: From 9356c217b2134114c32d6de31809ebb9e92e1da0 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 14:07:29 -0700 Subject: [PATCH 2/9] changed checking db schema length in test_api.py instead of checking if the db schema is the exact same, I am checking if it has more than 30 fields db schema is always changing so this test will break often, but it should have more than 30 fields because there are at least 24 nodes --- tests/api/test_api.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/api/test_api.py b/tests/api/test_api.py index 858245eeb..559d06f43 100644 --- a/tests/api/test_api.py +++ b/tests/api/test_api.py @@ -115,9 +115,8 @@ def test_get_db_schema_from_api(cript_api: cript.API) -> None: assert bool(db_schema) assert isinstance(db_schema, dict) - # TODO this is constantly changing, so we can't check it for now. - # total_fields_in_db_schema = 69 - # assert len(db_schema["$defs"]) == total_fields_in_db_schema + # db schema should have at least 30 fields + assert len(db_schema["$defs"]) > 30 def test_is_node_schema_valid(cript_api: cript.API) -> None: From eab0baf17835c78effe573eaa166be45611219c0 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 14:17:59 -0700 Subject: [PATCH 3/9] upgraded subobjects.py `simple_property_node` fixture * added docstrings * made instantiation into named arguements * changed tha variable name from `p` to `my_property` * still functions exactly the same --- tests/fixtures/subobjects.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/fixtures/subobjects.py b/tests/fixtures/subobjects.py index 9325bc61b..f3474c17d 100644 --- a/tests/fixtures/subobjects.py +++ b/tests/fixtures/subobjects.py @@ -165,13 +165,11 @@ def complex_property_dict(complex_material_node, complex_condition_dict, complex @pytest.fixture(scope="function") def simple_property_node() -> cript.Property: - p = cript.Property( - "modulus_shear", - "value", - 5.0, - "GPa", - ) - return p + """ + minimalist property node that only has the required attributes + """ + my_property = cript.Property(key="modulus_shear", type="value", value=5.0, unit="GPa") + return my_property @pytest.fixture(scope="function") From 18c6bf2b6744e8de689903db39cd89b4432ad308 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 15:02:49 -0700 Subject: [PATCH 4/9] renamed fixture * `complex_algorithm_node` was actually minimalistic with only required arguments instead of all arguments * all tests are passing as before --- tests/fixtures/subobjects.py | 24 ++++++++++--------- tests/nodes/subobjects/test_algorithm.py | 14 +++++------ tests/nodes/subobjects/test_parameter.py | 4 ++-- .../subobjects/test_software_configuration.py | 4 ++-- tests/test_node_util.py | 14 +++++------ 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/tests/fixtures/subobjects.py b/tests/fixtures/subobjects.py index f3474c17d..47ab20e19 100644 --- a/tests/fixtures/subobjects.py +++ b/tests/fixtures/subobjects.py @@ -26,7 +26,7 @@ def complex_parameter_dict() -> dict: # TODO this fixture should be renamed because it is simple_algorithm_subobject not complex @pytest.fixture(scope="function") -def complex_algorithm_node() -> cript.Algorithm: +def simple_algorithm_node() -> cript.Algorithm: """ minimal algorithm sub-object """ @@ -36,7 +36,7 @@ def complex_algorithm_node() -> cript.Algorithm: @pytest.fixture(scope="function") -def complex_algorithm_dict() -> dict: +def simple_algorithm_dict() -> dict: ret_dict = {"node": ["Algorithm"], "key": "mc_barostat", "type": "barostat"} return ret_dict @@ -165,11 +165,13 @@ def complex_property_dict(complex_material_node, complex_condition_dict, complex @pytest.fixture(scope="function") def simple_property_node() -> cript.Property: - """ - minimalist property node that only has the required attributes - """ - my_property = cript.Property(key="modulus_shear", type="value", value=5.0, unit="GPa") - return my_property + p = cript.Property( + "modulus_shear", + "value", + 5.0, + "GPa", + ) + return p @pytest.fixture(scope="function") @@ -322,20 +324,20 @@ def complex_computational_forcefield_dict(simple_data_node, complex_citation_dic @pytest.fixture(scope="function") -def complex_software_configuration_node(complex_software_node, complex_algorithm_node, complex_citation_node) -> cript.SoftwareConfiguration: +def complex_software_configuration_node(complex_software_node, simple_algorithm_node, complex_citation_node) -> cript.SoftwareConfiguration: """ maximal software_configuration sub-object with all possible attributes """ - my_complex_software_configuration_node = cript.SoftwareConfiguration(software=complex_software_node, algorithm=[complex_algorithm_node], notes="my_complex_software_configuration_node notes", citation=[complex_citation_node]) + my_complex_software_configuration_node = cript.SoftwareConfiguration(software=complex_software_node, algorithm=[simple_algorithm_node], notes="my_complex_software_configuration_node notes", citation=[complex_citation_node]) return my_complex_software_configuration_node @pytest.fixture(scope="function") -def complex_software_configuration_dict(complex_software_dict, complex_algorithm_dict, complex_citation_dict) -> dict: +def complex_software_configuration_dict(complex_software_dict, simple_algorithm_dict, complex_citation_dict) -> dict: ret_dict = { "node": ["SoftwareConfiguration"], "software": complex_software_dict, - "algorithm": [complex_algorithm_dict], + "algorithm": [simple_algorithm_dict], "notes": "my_complex_software_configuration_node notes", "citation": [complex_citation_dict], } diff --git a/tests/nodes/subobjects/test_algorithm.py b/tests/nodes/subobjects/test_algorithm.py index 7a27647af..949b1eb7c 100644 --- a/tests/nodes/subobjects/test_algorithm.py +++ b/tests/nodes/subobjects/test_algorithm.py @@ -7,8 +7,8 @@ import cript -def test_setter_getter(complex_algorithm_node, complex_citation_node): - a = complex_algorithm_node +def test_setter_getter(simple_algorithm_node, complex_citation_node): + a = simple_algorithm_node a.key = "berendsen" assert a.key == "berendsen" a.type = "integration" @@ -17,16 +17,16 @@ def test_setter_getter(complex_algorithm_node, complex_citation_node): assert strip_uid_from_dict(json.loads(a.citation[0].json)) == strip_uid_from_dict(json.loads(complex_citation_node.json)) -def test_json(complex_algorithm_node, complex_algorithm_dict, complex_citation_node): - a = complex_algorithm_node +def test_json(simple_algorithm_node, simple_algorithm_dict, complex_citation_node): + a = simple_algorithm_node a_dict = json.loads(a.json) - assert strip_uid_from_dict(a_dict) == complex_algorithm_dict + assert strip_uid_from_dict(a_dict) == simple_algorithm_dict print(a.get_json(indent=2).json) a2 = cript.load_nodes_from_json(a.json) assert strip_uid_from_dict(json.loads(a2.json)) == strip_uid_from_dict(a_dict) -def test_integration_algorithm(cript_api, simple_project_node, simple_collection_node, simple_experiment_node, simple_computation_node, simple_software_configuration, complex_algorithm_node): +def test_integration_algorithm(cript_api, simple_project_node, simple_collection_node, simple_experiment_node, simple_computation_node, simple_software_configuration, simple_algorithm_node): """ integration test between Python SDK and API Client @@ -42,7 +42,7 @@ def test_integration_algorithm(cript_api, simple_project_node, simple_collection simple_project_node.collection[0].experiment = [simple_experiment_node] simple_project_node.collection[0].experiment[0].computation = [simple_computation_node] simple_project_node.collection[0].experiment[0].computation[0].software_configuration = [simple_software_configuration] - simple_project_node.collection[0].experiment[0].computation[0].software_configuration[0].algorithm = [complex_algorithm_node] + simple_project_node.collection[0].experiment[0].computation[0].software_configuration[0].algorithm = [simple_algorithm_node] integrate_nodes_helper(cript_api=cript_api, project_node=simple_project_node) diff --git a/tests/nodes/subobjects/test_parameter.py b/tests/nodes/subobjects/test_parameter.py index 69287e8f0..ab782669b 100644 --- a/tests/nodes/subobjects/test_parameter.py +++ b/tests/nodes/subobjects/test_parameter.py @@ -26,7 +26,7 @@ def test_parameter_json_serialization(complex_parameter_node, complex_parameter_ assert p2.json == p.json -def test_integration_parameter(cript_api, simple_project_node, simple_collection_node, simple_experiment_node, simple_computation_node, simple_software_configuration, complex_algorithm_node, complex_parameter_node): +def test_integration_parameter(cript_api, simple_project_node, simple_collection_node, simple_experiment_node, simple_computation_node, simple_software_configuration, simple_algorithm_node, complex_parameter_node): """ integration test between Python SDK and API Client @@ -41,7 +41,7 @@ def test_integration_parameter(cript_api, simple_project_node, simple_collection simple_project_node.collection[0].experiment = [simple_experiment_node] simple_project_node.collection[0].experiment[0].computation = [simple_computation_node] simple_project_node.collection[0].experiment[0].computation[0].software_configuration = [simple_software_configuration] - simple_project_node.collection[0].experiment[0].computation[0].software_configuration[0].algorithm = [complex_algorithm_node] + simple_project_node.collection[0].experiment[0].computation[0].software_configuration[0].algorithm = [simple_algorithm_node] simple_project_node.collection[0].experiment[0].computation[0].software_configuration[0].algorithm[0].parameter = [complex_parameter_node] integrate_nodes_helper(cript_api=cript_api, project_node=simple_project_node) diff --git a/tests/nodes/subobjects/test_software_configuration.py b/tests/nodes/subobjects/test_software_configuration.py index 02a7b59d0..e3580d82b 100644 --- a/tests/nodes/subobjects/test_software_configuration.py +++ b/tests/nodes/subobjects/test_software_configuration.py @@ -17,14 +17,14 @@ def test_json(complex_software_configuration_node, complex_software_configuratio assert strip_uid_from_dict(json.loads(sc2.json)) == strip_uid_from_dict(json.loads(sc.json)) -def test_setter_getter(complex_software_configuration_node, complex_algorithm_node, complex_citation_node): +def test_setter_getter(complex_software_configuration_node, simple_algorithm_node, complex_citation_node): sc2 = complex_software_configuration_node software2 = copy.deepcopy(sc2.software) sc2.software = software2 assert sc2.software is software2 # assert len(sc2.algorithm) == 1 - # al2 = complex_algorithm_node + # al2 = simple_algorithm_node # print(sc2.get_json(indent=2,sortkeys=False).json) # print(al2.get_json(indent=2,sortkeys=False).json) # sc2.algorithm += [al2] diff --git a/tests/test_node_util.py b/tests/test_node_util.py index 37d7c70e4..882961baa 100644 --- a/tests/test_node_util.py +++ b/tests/test_node_util.py @@ -19,16 +19,16 @@ ) -def test_removing_nodes(complex_algorithm_node, complex_parameter_node, complex_algorithm_dict): - a = complex_algorithm_node +def test_removing_nodes(simple_algorithm_node, complex_parameter_node, simple_algorithm_dict): + a = simple_algorithm_node p = complex_parameter_node a.parameter += [p] - assert strip_uid_from_dict(json.loads(a.json)) != complex_algorithm_dict + assert strip_uid_from_dict(json.loads(a.json)) != simple_algorithm_dict a.remove_child(p) - assert strip_uid_from_dict(json.loads(a.json)) == complex_algorithm_dict + assert strip_uid_from_dict(json.loads(a.json)) == simple_algorithm_dict -def test_uid_deserialization(complex_algorithm_node, complex_parameter_node, complex_algorithm_dict): +def test_uid_deserialization(simple_algorithm_node, complex_parameter_node, simple_algorithm_dict): identifiers = [{"bigsmiles": "123456"}] material = cript.Material(name="my material", identifiers=identifiers) @@ -149,8 +149,8 @@ def test_json_error(complex_parameter_node): parameter.json -def test_local_search(complex_algorithm_node, complex_parameter_node): - a = complex_algorithm_node +def test_local_search(simple_algorithm_node, complex_parameter_node): + a = simple_algorithm_node # Check if we can use search to find the algorithm node, but specifying node and key find_algorithms = a.find_children({"node": "Algorithm", "key": "mc_barostat"}) assert find_algorithms == [a] From 1d6ba1ae5f689af0e36a36c8584311caf6141423 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 15:06:26 -0700 Subject: [PATCH 5/9] upgraded `simple_property_node` fixture --- tests/fixtures/subobjects.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/fixtures/subobjects.py b/tests/fixtures/subobjects.py index 47ab20e19..41eff8508 100644 --- a/tests/fixtures/subobjects.py +++ b/tests/fixtures/subobjects.py @@ -165,13 +165,13 @@ def complex_property_dict(complex_material_node, complex_condition_dict, complex @pytest.fixture(scope="function") def simple_property_node() -> cript.Property: - p = cript.Property( - "modulus_shear", - "value", - 5.0, - "GPa", + my_property = cript.Property( + key="modulus_shear", + type="value", + value=5.0, + unit="GPa", ) - return p + return my_property @pytest.fixture(scope="function") From fa224a2617e3e9ae615267349e07ecc53d41ca37 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 15:41:17 -0700 Subject: [PATCH 6/9] cleaned up `complex_process_node` fixture * avoiding `deep_copy` as that causes issues * using simple fixtures * to avoid deep_copy and to make working with a huge node easier * using fixtures instead of remaking nodes * `complex_process` fixture is not being used in any tests --- tests/fixtures/primary_nodes.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/fixtures/primary_nodes.py b/tests/fixtures/primary_nodes.py index 40eb970cb..0bdc94208 100644 --- a/tests/fixtures/primary_nodes.py +++ b/tests/fixtures/primary_nodes.py @@ -163,7 +163,7 @@ def simple_process_node() -> cript.Process: @pytest.fixture(scope="function") -def complex_process_node(complex_ingredient_node, simple_equipment_node, complex_citation_node, simple_property_node, simple_condition_node, simple_material_node, simple_process_node, complex_equipment_node, complex_condition_node) -> None: +def complex_process_node(complex_ingredient_node, simple_equipment_node, complex_citation_node, simple_property_node, simple_condition_node, simple_material_node, simple_process_node) -> None: """ create a process node with all possible arguments @@ -171,7 +171,6 @@ def complex_process_node(complex_ingredient_node, simple_equipment_node, complex ----- * indirectly tests the vocabulary as well, as it gives it valid vocabulary """ - # TODO clean up this test and use fixtures from conftest.py my_process_name = "my complex process node name" my_process_type = "affinity_pure" @@ -186,23 +185,19 @@ def complex_process_node(complex_ingredient_node, simple_equipment_node, complex "annealing_sol", ] - # create complex process - citation = copy.deepcopy(complex_citation_node) - prop = cript.Property("n_neighbor", "value", 2.0, None) - my_complex_process = cript.Process( name=my_process_name, type=my_process_type, ingredient=[complex_ingredient_node], description=my_process_description, - equipment=[complex_equipment_node], + equipment=[simple_equipment_node], product=[simple_material_node], waste=process_waste, prerequisite_process=[simple_process_node], - condition=[complex_condition_node], - property=[prop], + condition=[simple_condition_node], + property=[simple_property_node], keyword=my_process_keywords, - citation=[citation], + citation=[complex_citation_node], ) return my_complex_process From f4264d15bb6f9875e8c83dea76ec6b2042533160 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 15:57:06 -0700 Subject: [PATCH 7/9] knocking out TODO in file.py --- src/cript/nodes/supporting_nodes/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cript/nodes/supporting_nodes/file.py b/src/cript/nodes/supporting_nodes/file.py index 3ef367be1..5c75cfa5d 100644 --- a/src/cript/nodes/supporting_nodes/file.py +++ b/src/cript/nodes/supporting_nodes/file.py @@ -409,7 +409,7 @@ def data_dictionary(self, new_data_dictionary: str) -> None: new_attrs = replace(self._json_attrs, data_dictionary=new_data_dictionary) self._update_json_attrs_if_valid(new_attrs) - # TODO get file name from node itself as default and allow for customization as well optional + @beartype def download( self, destination_directory_path: Union[str, Path] = ".", From 3bb685ef8fd25034a009565ae41cd9dd50918c91 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 16:09:36 -0700 Subject: [PATCH 8/9] allowing for Path object in api constructor for config file * removed unneeded comment --- src/cript/api/api.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cript/api/api.py b/src/cript/api/api.py index c078e5d3a..caeef8c23 100644 --- a/src/cript/api/api.py +++ b/src/cript/api/api.py @@ -74,7 +74,7 @@ class API: # trunk-ignore-end(cspell) @beartype - def __init__(self, host: Union[str, None] = None, api_token: Union[str, None] = None, storage_token: Union[str, None] = None, config_file_path: str = ""): + def __init__(self, host: Union[str, None] = None, api_token: Union[str, None] = None, storage_token: Union[str, None] = None, config_file_path: Union[str, Path] = ""): """ Initialize CRIPT API client with host and token. Additionally, you can use a config.json file and specify the file path. @@ -164,8 +164,7 @@ def __init__(self, host: Union[str, None] = None, api_token: Union[str, None] = self._api_token = api_token # type: ignore self._storage_token = storage_token # type: ignore - # assign headers - # add Bearer to token for HTTP, but keep it bare for AWS S3 file uploads and downloads + # add Bearer to token for HTTP requests self._http_headers = {"Authorization": f"Bearer {self._api_token}", "Content-Type": "application/json"} # check that api can connect to CRIPT with host and token From 48492ee37a12c3524024d91ce464c972d5c0ca98 Mon Sep 17 00:00:00 2001 From: nh916 Date: Fri, 21 Jul 2023 16:37:58 -0700 Subject: [PATCH 9/9] removing `dee_copy` from `simple_process_node` simple_process_node does not need deep_copy within the fixture as is more straight forward and all tests work fine without it and --- tests/fixtures/primary_nodes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/primary_nodes.py b/tests/fixtures/primary_nodes.py index 0bdc94208..04b219c02 100644 --- a/tests/fixtures/primary_nodes.py +++ b/tests/fixtures/primary_nodes.py @@ -159,7 +159,7 @@ def simple_process_node() -> cript.Process: """ my_process = cript.Process(name="my process name", type="affinity_pure") - return copy.deepcopy(my_process) + return my_process @pytest.fixture(scope="function")