From e59475c90b3c2e8f7028e17fab1127a113744e15 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Fri, 1 Sep 2023 10:56:18 -0700 Subject: [PATCH 01/12] raise exception when node cant be found in schema --- schematic/manifest/generator.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/schematic/manifest/generator.py b/schematic/manifest/generator.py index 3231c516b..03f42ec58 100644 --- a/schematic/manifest/generator.py +++ b/schematic/manifest/generator.py @@ -83,7 +83,14 @@ def __init__( # Determine whether current data type is file-based is_file_based = False if self.root: - is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) + try: + is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) + except KeyError as e: + if self.root in str(e): + raise LookupError(f"The DataType entered ({self.root}) could not be found in the data model schema. Please confirm that the datatype is in the data model and that the spelling matches what is in the .jsonld file.") from e + else: + raise(e) + self.is_file_based = is_file_based def _attribute_to_letter(self, attribute, manifest_fields): From 5dd18a2393af80a9db80a75cb5df745e90a894a7 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Fri, 1 Sep 2023 11:27:37 -0700 Subject: [PATCH 02/12] update formatting and message --- schematic/manifest/generator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/schematic/manifest/generator.py b/schematic/manifest/generator.py index 03f42ec58..24a18c4c3 100644 --- a/schematic/manifest/generator.py +++ b/schematic/manifest/generator.py @@ -87,7 +87,9 @@ def __init__( is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) except KeyError as e: if self.root in str(e): - raise LookupError(f"The DataType entered ({self.root}) could not be found in the data model schema. Please confirm that the datatype is in the data model and that the spelling matches what is in the .jsonld file.") from e + exception_message = f"The DataType entered ({self.root}) could not be found in the data model schema. " + \ + "Please confirm that the datatype is in the data model and that the spelling matches the class label in the .jsonld file." + raise LookupError(exception_message) from e else: raise(e) From 0f95ba96c99627060b833aef10397ee2b133fbd4 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Thu, 7 Sep 2023 10:12:00 -0700 Subject: [PATCH 03/12] check if class is in schema instead of `try: except:` --- schematic/manifest/generator.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/schematic/manifest/generator.py b/schematic/manifest/generator.py index 24a18c4c3..388b665f6 100644 --- a/schematic/manifest/generator.py +++ b/schematic/manifest/generator.py @@ -82,16 +82,18 @@ def __init__( # Determine whether current data type is file-based is_file_based = False - if self.root: - try: - is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) - except KeyError as e: - if self.root in str(e): - exception_message = f"The DataType entered ({self.root}) could not be found in the data model schema. " + \ - "Please confirm that the datatype is in the data model and that the spelling matches the class label in the .jsonld file." - raise LookupError(exception_message) from e - else: - raise(e) + + if self.root: + # Check if the class is in the schema + root_in_schema = self.sg.se.is_class_in_schema(self.root) + + # If the class could not be found, give a notification + if not root_in_schema: + exception_message = f"The DataType entered ({self.root}) could not be found in the data model schema. " + \ + "Please confirm that the datatype is in the data model and that the spelling matches the class label in the .jsonld file." + raise LookupError(exception_message) + + is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) self.is_file_based = is_file_based From 40e1dbf48e517e02b62542107c2be01cbce3fbe6 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Thu, 14 Sep 2023 11:27:48 -0700 Subject: [PATCH 04/12] add test for exception --- tests/test_manifest.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 12d9e9474..2132fb560 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -94,6 +94,24 @@ def test_init(self, helpers): assert generator.root is None assert type(generator.sg) is SchemaGenerator + def test_missing_root_error(self, helpers): + """ + The ManifestGenerator should raise a LookupError during initialization if a data_type is passed in that can't be found in the schema. + """ + # Choose a data type that is not in the schema + data_type = "MissingComponent" + + # A LookupError should be raised and include message when the component cannot be found + with pytest.raises(LookupError) as e: + generator = ManifestGenerator( + path_to_json_ld=helpers.get_data_path("example.model.jsonld"), + root=data_type, + use_annotations=False, + ) + + # Check message contents + assert f"({data_type}) could not be found in the data model schema" in str(e) + @pytest.mark.google_credentials_needed def test_get_manifest_first_time(self, manifest): @@ -145,7 +163,7 @@ def test_get_manifest_first_time(self, manifest): # An annotation merged with an attribute from the data model if use_annotations: assert output["File Format"].tolist() == ["txt", "csv", "fastq"] - + @pytest.mark.parametrize("output_format", [None, "dataframe", "excel", "google_sheet"]) @pytest.mark.parametrize("sheet_url", [None, True, False]) @pytest.mark.parametrize("dataset_id", [None, "syn27600056"]) @@ -381,5 +399,3 @@ def test_populate_existing_excel_spreadsheet(self, simple_manifest_generator, si # remove file os.remove(dummy_output_path) - - From 9ea0fde0b2939d0f4e415ce64510e09693c186ca Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:26:58 -0700 Subject: [PATCH 05/12] change legacy logic for no root provided --- schematic/manifest/generator.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/schematic/manifest/generator.py b/schematic/manifest/generator.py index 388b665f6..389a38c04 100644 --- a/schematic/manifest/generator.py +++ b/schematic/manifest/generator.py @@ -80,22 +80,22 @@ def __init__( # additional metadata to add to manifest self.additional_metadata = additional_metadata - # Determine whether current data type is file-based - is_file_based = False + # Raise an error if no DataType has been provided + if not self.root: + raise ValueError("No DataType has been provided.") + + # Check if the class is in the schema + root_in_schema = self.sg.se.is_class_in_schema(self.root) + + # If the class could not be found, give a notification + if not root_in_schema: + exception_message = f"The DataType entered ({self.root}) could not be found in the data model schema. " + \ + "Please confirm that the datatype is in the data model and that the spelling matches the class label in the .jsonld file." + raise LookupError(exception_message) - if self.root: - # Check if the class is in the schema - root_in_schema = self.sg.se.is_class_in_schema(self.root) - - # If the class could not be found, give a notification - if not root_in_schema: - exception_message = f"The DataType entered ({self.root}) could not be found in the data model schema. " + \ - "Please confirm that the datatype is in the data model and that the spelling matches the class label in the .jsonld file." - raise LookupError(exception_message) - - is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) + # Determine whether current data type is file-based + self.is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) - self.is_file_based = is_file_based def _attribute_to_letter(self, attribute, manifest_fields): """Map attribute to column letter in a google sheet""" From 65505686534ac8225ccc1efa5fc6071be839dfe9 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:37:31 -0700 Subject: [PATCH 06/12] update test to check for both exceptions --- tests/test_manifest.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 2132fb560..5b5bba17c 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -94,15 +94,17 @@ def test_init(self, helpers): assert generator.root is None assert type(generator.sg) is SchemaGenerator - def test_missing_root_error(self, helpers): + @pytest.mark.parametrize("data_type, exc, exc_message", + [("MissingComponent", LookupError, "could not be found in the data model schema"), + (None, ValueError, "No DataType has been provided.")], + ids = ["DataType not found in Schema", "No DataType provided"]) + def test_missing_root_error(self, helpers, data_type, exc, exc_message): """ The ManifestGenerator should raise a LookupError during initialization if a data_type is passed in that can't be found in the schema. """ - # Choose a data type that is not in the schema - data_type = "MissingComponent" # A LookupError should be raised and include message when the component cannot be found - with pytest.raises(LookupError) as e: + with pytest.raises(exc) as e: generator = ManifestGenerator( path_to_json_ld=helpers.get_data_path("example.model.jsonld"), root=data_type, @@ -110,7 +112,7 @@ def test_missing_root_error(self, helpers): ) # Check message contents - assert f"({data_type}) could not be found in the data model schema" in str(e) + assert exc_message in str(e) @pytest.mark.google_credentials_needed def test_get_manifest_first_time(self, manifest): From 9f5a1745a3420b8f1f461f5bf3940c1c563b2f95 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:40:34 -0700 Subject: [PATCH 07/12] reorder logic --- schematic/manifest/generator.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/schematic/manifest/generator.py b/schematic/manifest/generator.py index 389a38c04..e2dc0e9f3 100644 --- a/schematic/manifest/generator.py +++ b/schematic/manifest/generator.py @@ -54,7 +54,11 @@ def __init__( self.creds = services_creds["creds"] # schema root - self.root = root + if root: + self.root = root + # Raise an error if no DataType has been provided + else: + raise ValueError("No DataType has been provided.") # alphabetize valid values self.alphabetize = alphabetize_valid_values @@ -79,10 +83,6 @@ def __init__( # additional metadata to add to manifest self.additional_metadata = additional_metadata - - # Raise an error if no DataType has been provided - if not self.root: - raise ValueError("No DataType has been provided.") # Check if the class is in the schema root_in_schema = self.sg.se.is_class_in_schema(self.root) From dd2721fda4fd5500ea0db4d3892bb4dc609e6c17 Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:42:41 -0700 Subject: [PATCH 08/12] update docstring --- tests/test_manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 5b5bba17c..2fa07f71d 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -100,7 +100,7 @@ def test_init(self, helpers): ids = ["DataType not found in Schema", "No DataType provided"]) def test_missing_root_error(self, helpers, data_type, exc, exc_message): """ - The ManifestGenerator should raise a LookupError during initialization if a data_type is passed in that can't be found in the schema. + Test for errors when either no DataType is provided or when a DataType is provided but not found in the schema """ # A LookupError should be raised and include message when the component cannot be found From b0799cfce492296054d3284e04960d9b671e857d Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Thu, 21 Sep 2023 09:34:41 -0700 Subject: [PATCH 09/12] update `test_init` to pass in root --- tests/test_manifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 2fa07f71d..880deb179 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -87,11 +87,12 @@ def test_init(self, helpers): generator = ManifestGenerator( title="mock_title", path_to_json_ld=helpers.get_data_path("example.model.jsonld"), + root = "Patient" ) assert type(generator.title) is str # assert generator.sheet_service == mock_creds["sheet_service"] - assert generator.root is None + assert generator.root is "Patient" assert type(generator.sg) is SchemaGenerator @pytest.mark.parametrize("data_type, exc, exc_message", From e4d0cebb186eff60b44f5ad4062767fddc1cc9d8 Mon Sep 17 00:00:00 2001 From: andrewelamb Date: Thu, 21 Sep 2023 11:52:42 -0700 Subject: [PATCH 10/12] redid poetry install in publish workflow --- .github/workflows/publish.yml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d77016a91..05a99315c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -20,15 +20,16 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} - + #---------------------------------------------- - # install & configure poetry + # install & configure poetry #---------------------------------------------- - name: Install Poetry - uses: snok/install-poetry@v1.1.1 - with: - virtualenvs-create: true - virtualenvs-in-project: true + run: | + curl -sSL https://install.python-poetry.org \ + | python3 - --version ${{ env.POETRY_VERSION }}; + poetry config virtualenvs.create true; + poetry config virtualenvs.in-project true; #---------------------------------------------- # load cached venv if cache exists From 492fbf112b4aa0973de5c811ef4bfde8216ab0cb Mon Sep 17 00:00:00 2001 From: Gianna Jordan <61707471+GiaJordan@users.noreply.github.com> Date: Thu, 21 Sep 2023 14:12:40 -0700 Subject: [PATCH 11/12] Revert "Raise a helpful error when a datatype cannot be found in a schema" --- schematic/manifest/generator.py | 21 +++++---------------- tests/test_manifest.py | 27 ++++----------------------- 2 files changed, 9 insertions(+), 39 deletions(-) diff --git a/schematic/manifest/generator.py b/schematic/manifest/generator.py index 778ba411d..c9cb81cdf 100644 --- a/schematic/manifest/generator.py +++ b/schematic/manifest/generator.py @@ -54,11 +54,7 @@ def __init__( self.creds = services_creds["creds"] # schema root - if root: - self.root = root - # Raise an error if no DataType has been provided - else: - raise ValueError("No DataType has been provided.") + self.root = root # alphabetize valid values self.alphabetize = alphabetize_valid_values @@ -83,19 +79,12 @@ def __init__( # additional metadata to add to manifest self.additional_metadata = additional_metadata - - # Check if the class is in the schema - root_in_schema = self.sg.se.is_class_in_schema(self.root) - - # If the class could not be found, give a notification - if not root_in_schema: - exception_message = f"The DataType entered ({self.root}) could not be found in the data model schema. " + \ - "Please confirm that the datatype is in the data model and that the spelling matches the class label in the .jsonld file." - raise LookupError(exception_message) # Determine whether current data type is file-based - self.is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) - + is_file_based = False + if self.root: + is_file_based = "Filename" in self.sg.get_node_dependencies(self.root) + self.is_file_based = is_file_based def _attribute_to_letter(self, attribute, manifest_fields): """Map attribute to column letter in a google sheet""" diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 0eb905cf1..21f2f4285 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -87,34 +87,13 @@ def test_init(self, helpers): generator = ManifestGenerator( title="mock_title", path_to_json_ld=helpers.get_data_path("example.model.jsonld"), - root = "Patient" ) assert type(generator.title) is str # assert generator.sheet_service == mock_creds["sheet_service"] - assert generator.root is "Patient" + assert generator.root is None assert type(generator.sg) is SchemaGenerator - @pytest.mark.parametrize("data_type, exc, exc_message", - [("MissingComponent", LookupError, "could not be found in the data model schema"), - (None, ValueError, "No DataType has been provided.")], - ids = ["DataType not found in Schema", "No DataType provided"]) - def test_missing_root_error(self, helpers, data_type, exc, exc_message): - """ - Test for errors when either no DataType is provided or when a DataType is provided but not found in the schema - """ - - # A LookupError should be raised and include message when the component cannot be found - with pytest.raises(exc) as e: - generator = ManifestGenerator( - path_to_json_ld=helpers.get_data_path("example.model.jsonld"), - root=data_type, - use_annotations=False, - ) - - # Check message contents - assert exc_message in str(e) - @pytest.mark.google_credentials_needed def test_get_manifest_first_time(self, manifest): @@ -166,7 +145,7 @@ def test_get_manifest_first_time(self, manifest): # An annotation merged with an attribute from the data model if use_annotations: assert output["File Format"].tolist() == ["txt", "csv", "fastq"] - + @pytest.mark.parametrize("output_format", [None, "dataframe", "excel", "google_sheet"]) @pytest.mark.parametrize("sheet_url", [None, True, False]) @pytest.mark.parametrize("dataset_id", [None, "syn27600056"]) @@ -433,3 +412,5 @@ def test_populate_existing_excel_spreadsheet(self, simple_manifest_generator, si # remove file os.remove(dummy_output_path) + + From 7c71206ebd764645cb2c3fcfa35ec0a5656d3925 Mon Sep 17 00:00:00 2001 From: andrewelamb Date: Thu, 21 Sep 2023 14:27:09 -0700 Subject: [PATCH 12/12] add poetry verison as env variable --- .github/workflows/publish.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 05a99315c..2255a52e2 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -8,6 +8,8 @@ on: jobs: pypi_release: runs-on: ubuntu-latest + env: + POETRY_VERSION: 1.2.0 if: github.event_name == 'push' && contains(github.ref, 'refs/tags') steps: #----------------------------------------------