From 43d0e5e43370ef8dfce6bd625849e0e30b7088eb Mon Sep 17 00:00:00 2001 From: Joyce Yan <5653616+joyceyan@users.noreply.github.com> Date: Tue, 26 Sep 2023 16:47:16 -0400 Subject: [PATCH 1/3] only validate colors if the key is specified --- .../cellxgene_schema/validate.py | 20 ++++++++++--------- .../tests/test_schema_compliance.py | 4 ++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cellxgene_schema_cli/cellxgene_schema/validate.py b/cellxgene_schema_cli/cellxgene_schema/validate.py index 61ee83e9b..e423b0915 100644 --- a/cellxgene_schema_cli/cellxgene_schema/validate.py +++ b/cellxgene_schema_cli/cellxgene_schema/validate.py @@ -696,17 +696,19 @@ def _validate_colors_in_uns_dict(self, uns_dict: dict) -> None: for column_name, num_unique_vals in category_mapping.items(): colors_options = uns_dict.get(f"{column_name}_colors", []) - if len(colors_options) < num_unique_vals: - self.errors.append( - f"Annotated categorical field {column_name} must have at least {num_unique_vals} color options " - f"in uns[{column_name}_colors]. Found: {colors_options}" - ) - for color in colors_options: - if not self._validate_color(color): + # If there are no colors specified, that's fine. We only want to validate this field if it's set + if len(colors_options) > 0: + if len(colors_options) < num_unique_vals: self.errors.append( - f"Color {color} in uns[{column_name}_colors] is not valid. Colors must be a valid hex " - f"code (#08c0ff) or a CSS4 named color" + f"Annotated categorical field {column_name} must have at least {num_unique_vals} color options " + f"in uns[{column_name}_colors]. Found: {colors_options}" ) + for color in colors_options: + if not self._validate_color(color): + self.errors.append( + f"Color {color} in uns[{column_name}_colors] is not valid. Colors must be a valid hex " + f"code (#08c0ff) or a CSS4 named color" + ) def _validate_color(self, color: str) -> bool: css4_named_colors = [ diff --git a/cellxgene_schema_cli/tests/test_schema_compliance.py b/cellxgene_schema_cli/tests/test_schema_compliance.py index 2a9ef0706..7c7692b57 100644 --- a/cellxgene_schema_cli/tests/test_schema_compliance.py +++ b/cellxgene_schema_cli/tests/test_schema_compliance.py @@ -1359,6 +1359,10 @@ def test_deprecated_fields(self): "ERROR: The field 'publication_doi' is present in 'uns', but it is deprecated.", ], ) + + def test_no_colors_should_pass(self): + del self.validator.adata.uns["suspension_type_colors"] + self.assertTrue(self.validator.validate_adata()) def test_not_enough_color_options(self): self.validator.adata.uns["suspension_type_colors"] = ["green"] From f7dee49e379429ddc1362498605d736f5e5a4604 Mon Sep 17 00:00:00 2001 From: Joyce Yan <5653616+joyceyan@users.noreply.github.com> Date: Tue, 26 Sep 2023 16:56:10 -0400 Subject: [PATCH 2/3] fix validation to only unset keys --- cellxgene_schema_cli/cellxgene_schema/validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cellxgene_schema_cli/cellxgene_schema/validate.py b/cellxgene_schema_cli/cellxgene_schema/validate.py index e423b0915..13a9bdaac 100644 --- a/cellxgene_schema_cli/cellxgene_schema/validate.py +++ b/cellxgene_schema_cli/cellxgene_schema/validate.py @@ -695,9 +695,9 @@ def _validate_colors_in_uns_dict(self, uns_dict: dict) -> None: category_mapping[column_name] = df[column_name].nunique() for column_name, num_unique_vals in category_mapping.items(): - colors_options = uns_dict.get(f"{column_name}_colors", []) + colors_options = uns_dict.get(f"{column_name}_colors") # If there are no colors specified, that's fine. We only want to validate this field if it's set - if len(colors_options) > 0: + if colors_options is not None: if len(colors_options) < num_unique_vals: self.errors.append( f"Annotated categorical field {column_name} must have at least {num_unique_vals} color options " From 1aa127d0cd2beefe9fbe0d2c68db65e42492e629 Mon Sep 17 00:00:00 2001 From: Joyce Yan <5653616+joyceyan@users.noreply.github.com> Date: Tue, 26 Sep 2023 16:56:35 -0400 Subject: [PATCH 3/3] fix linter --- cellxgene_schema_cli/tests/test_schema_compliance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cellxgene_schema_cli/tests/test_schema_compliance.py b/cellxgene_schema_cli/tests/test_schema_compliance.py index 7c7692b57..c2e4ff701 100644 --- a/cellxgene_schema_cli/tests/test_schema_compliance.py +++ b/cellxgene_schema_cli/tests/test_schema_compliance.py @@ -1359,7 +1359,7 @@ def test_deprecated_fields(self): "ERROR: The field 'publication_doi' is present in 'uns', but it is deprecated.", ], ) - + def test_no_colors_should_pass(self): del self.validator.adata.uns["suspension_type_colors"] self.assertTrue(self.validator.validate_adata())