Skip to content

Commit

Permalink
Merge pull request #1442 from Sage-Bionetworks/develop-add-black-test…
Browse files Browse the repository at this point in the history
…-api

technical debt: run black for schematic api and test and modify pre-commit
  • Loading branch information
linglp authored Jun 19, 2024
2 parents cb17c5b + 0ea1aec commit ddfb9d5
Show file tree
Hide file tree
Showing 13 changed files with 566 additions and 273 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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/
files: ^(tests|schematic|schematic_api)/
5 changes: 3 additions & 2 deletions schematic_api/api/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from schematic_api.api import app


def main():
def main():
# Get app configuration
host = os.environ.get("APP_HOST", "0.0.0.0")
port = os.environ.get("APP_PORT", "3001")
Expand All @@ -11,5 +11,6 @@ def main():
# Launch app
app.run(host=host, port=port, debug=False)


if __name__ == "__main__":
main()
main()
13 changes: 9 additions & 4 deletions schematic_api/api/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
return version
2 changes: 1 addition & 1 deletion schematic_api/api/security_controller_.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
return {"uid": "user_id"}
11 changes: 7 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 14 additions & 7 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -143,15 +143,15 @@ 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

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
Expand Down Expand Up @@ -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
)
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down
129 changes: 99 additions & 30 deletions tests/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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

Loading

0 comments on commit ddfb9d5

Please sign in to comment.