Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

technical debt: run black for schematic api and test and modify pre-commit #1442

Merged
merged 4 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

6 changes: 4 additions & 2 deletions tests/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading