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

PR suggestion: Remove graph_data_model parameter in create_manifests to simplify the code #1499

Open
wants to merge 5 commits into
base: FDS-1797-input-graph-api
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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 .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ jobs:


- name: Upload pytest test results
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: pytest-results-${{ matrix.python-version }}
path: htmlcov
Expand Down
32 changes: 12 additions & 20 deletions schematic/manifest/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@
execute_google_api_requests,
export_manifest_drive_service,
)
from schematic.utils.io_utils import read_pickle
from schematic.utils.schema_utils import (
DisplayLabelType,
extract_component_validation_rules,
)
from schematic.utils.df_utils import update_df, load_df
from schematic.utils.io_utils import read_pickle
from schematic.utils.validate_utils import rule_in_rule_list

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -1662,13 +1661,11 @@ def create_manifests(
title: Optional[str] = None,
strict: Optional[bool] = True,
use_annotations: Optional[bool] = False,
graph_data_model: Optional[nx.MultiDiGraph] = None,
data_model_graph_pickle: Optional[str] = None,
) -> Union[List[str], List[pd.DataFrame]]:
"""Create multiple manifests

Args:
path_to_data_model (str): str path to data model
data_model_graph_pickle (str, optional): path to pickled networkx MultiDiGraph object. Defaults to None.
graph_data_model (str, optional): An networkx MultiDiGraph object. Defaults to None.
data_types (list): a list of data types
Expand Down Expand Up @@ -1728,25 +1725,20 @@ def create_manifests(
"Please check your submission and try again."
)

if graph_data_model is None:
if data_model_graph_pickle:
"""What if pickle file does not fit in memory?"""
graph_data_model = read_pickle(data_model_graph_pickle)
else:
data_model_parser = DataModelParser(
path_to_data_model=path_to_data_model
)
if data_model_graph_pickle:
"""What if pickle file does not fit in memory?"""
graph_data_model = read_pickle(data_model_graph_pickle)
else:
data_model_parser = DataModelParser(path_to_data_model=path_to_data_model)

# Parse Model
parsed_data_model = data_model_parser.parse_model()
# Parse Model
parsed_data_model = data_model_parser.parse_model()

# Instantiate DataModelGraph
data_model_grapher = DataModelGraph(
parsed_data_model, data_model_labels
)
# Instantiate DataModelGraph
data_model_grapher = DataModelGraph(parsed_data_model, data_model_labels)

# Generate graph
graph_data_model = data_model_grapher.graph
# Generate graph
graph_data_model = data_model_grapher.graph

# Gather all returned result urls
all_results = []
Expand Down
13 changes: 5 additions & 8 deletions schematic_api/api/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
from schematic.schemas.data_model_parser import DataModelParser
from schematic.store.synapse import ManifestDownload, SynapseStorage
from schematic.utils.general import entity_type_mapping
from schematic.utils.io_utils import read_pickle
from schematic.utils.schema_utils import (
DisplayLabelType,
get_property_label_from_display_name,
)
from schematic.utils.io_utils import read_pickle
from schematic.visualization.attributes_explorer import AttributesExplorer
from schematic.visualization.tangled_tree import TangledTree

Expand Down Expand Up @@ -292,10 +292,8 @@ def initalize_metadata_model(schema_url, data_model_labels):
)
return metadata_model

def get_temp_file(
url: str,
suffix: str
) -> str:

def get_temp_file(url: str, suffix: str) -> str:
"""
Retrieve a file via URL and store it in a temporary location
:param url str: URL to the file
Expand All @@ -308,6 +306,7 @@ def get_temp_file(

return tmp_file.name


def get_temp_model_path(schema_url):
# Get model type:
model_extension = pathlib.Path(schema_url).suffix.replace(".", "").upper()
Expand Down Expand Up @@ -357,10 +356,8 @@ def get_manifest_route(

config_handler(asset_view=asset_view)

graph_data_model = None
if graph_url is not None:
graph_path = get_temp_model_path(graph_url)
graph_data_model = read_pickle(graph_path)

all_results = ManifestGenerator.create_manifests(
path_to_data_model=schema_url,
Expand All @@ -372,7 +369,7 @@ def get_manifest_route(
strict=strict_validation,
use_annotations=use_annotations,
data_model_labels=data_model_labels,
graph_data_model=graph_data_model,
data_model_graph_pickle=graph_path,
)

# return an excel file if output_format is set to "excel"
Expand Down
17 changes: 9 additions & 8 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import json
import os

import pytest
import pickle
import json
from unittest.mock import patch

import pytest
from click.testing import CliRunner

from schematic.schemas.commands import schema
from schematic.configuration.configuration import Configuration
from schematic.manifest.commands import manifest
from schematic.models.commands import model
from schematic.configuration.configuration import Configuration
from schematic.schemas.commands import schema
from tests.conftest import Helpers


Expand Down Expand Up @@ -52,9 +51,10 @@ def assert_expected_file(self, result, output_path):
"output_path",
[
# Test case 1: pickle file passed to output_path
"tests/data/example.model.pickle",
"tests/data/example.model.pickle",
# Test case 2: jsonld file passed to output_path
"tests/data/example.model.jsonld"],
"tests/data/example.model.jsonld",
],
ids=["output_path_pickle", "output_path_jsonld"],
)
@pytest.mark.parametrize(
Expand All @@ -65,7 +65,8 @@ def assert_expected_file(self, result, output_path):
# Test case 2: graph passed to output_type
"graph",
# Test case 3: both jsonld and graph are created
"all"],
"all",
],
ids=["output_type_jsonld", "output_type_graph", "output_type_all"],
)
def test_schema_convert_cli(self, runner, output_path, output_type):
Expand Down
Loading