[GEN-2407] Add to genie cli to skip all validation rules requiring internal database access for local validator (#628)

* add new parameter skip_database_checks to handle all internal database validation checks

* update local validation instruction, remove duplication and add to docstring

* refactor cna validation

* add numbers

* lint

* update docstring

* lint

Author: rxu17

README.md

@@ -13,9 +13,6 @@
 - [Documentation](#documentation)
 - [Dependencies](#dependencies)
 - [File Validator](#file-validator)
-  - [Setting up your environment](#setting-up-your-environment)
-  - [Running the validator](#running-the-validator)
-  - [Example commands](#example-commands)
 - [Contributing](#contributing)
 - [Sage Bionetworks Only](#sage-bionetworks-only)
   - [Running locally](#running-locally)
@@ -57,57 +54,7 @@ This package contains both R, Python and cli tools.  These are tools or packages
 
 ## File Validator
 
-One of the features of the `aacrgenie` package is that is provides a local validation tool that GENIE data contributors and install and use to validate their files locally prior to uploading to Synapse.
-
-
-### Setting up your environment
-
-These instructions will install all the necessary components for you to run the validator locally on all of your files, including the Synapse client.
-
-1. Create a virtual environment using package manager of your choice (e.g: `conda`, `pipenv`, `pip`)
-
-Example of creating a simple python environment
-
-```
-python3 -m venv <env_name>
-source <env_name>/bin/activate
-```
-
-2. Install the genie package
-
-```
-pip install aacrgenie
-```
-
-3. Verify the installation
-
-```
-genie -v
-```
-
-4. Set up authentication with Synapse through the [local .synapseConfig](https://python-docs.synapse.org/tutorials/authentication/#use-synapseconfig) or using an [environment variable](https://python-docs.synapse.org/tutorials/authentication/#use-environment-variable)
-
-### Running the validator
-
-Get help of all available commands
-
-```
-genie validate -h
-```
-
-### Example commands
-
-Running validator on clinical file
-
-```
-genie validate data_clinical_supp_SAGE.txt SAGE
-```
-
-Running validator on cna file. **Note** that the flag `--nosymbol-check` is **REQUIRED** when running the validator for cna files because you would need access to an internal bed database table without it. For DEVELOPERS this is not required.
-
-```
-genie validate data_cna_SAGE.txt SAGE --nosymbol-check
-```
+Please see the [local file validation tutorial](/docs/tutorials/local_file_validation.md) for more information on this and how to use it.
 
 ## Contributing
 

docs/tutorials/local_file_validation.md

@@ -2,12 +2,39 @@
 
 One of the features of the `aacrgenie` package is that is provides a local validation tool that GENIE data contributors and install and use to validate their files locally prior to uploading to Synapse.
 
+## Setting up your environment
+
+These instructions will install all the necessary components for you to run the validator locally on all of your files, including the Synapse client.
+
+1. Create a virtual environment using package manager of your choice (e.g: `conda`, `pipenv`, `pip`)
+
+Example of creating a simple python environment
+
+```bash
+python3 -m venv <env_name>
+source <env_name>/bin/activate
 ```
+
+2. Install the genie package
+
+```bash
 pip install aacrgenie
+```
+
+3. Verify the installation
+
+```bash
 genie -v
 ```
 
-This will install all the necessary components for you to run the validator locally on all of your files, including the Synapse client.  Please view the help to see how to run to validator.
+4. Set up authentication with Synapse through the [local .synapseConfig](https://python-docs.synapse.org/tutorials/authentication/#use-synapseconfig) or using an [environment variable](https://python-docs.synapse.org/tutorials/authentication/#use-environment-variable)
+
+
+This will install all the necessary components for you to run the validator locally on all of your files, including the Synapse client.
+
+## Running the validator
+
+Please view the help to see how to run the validator.
 
 ```
 genie validate -h
@@ -18,3 +45,22 @@ Validate a file
 ```
 genie validate data_clinical_supp_SAGE.txt SAGE
 ```
+
+### Special Consideration
+
+The flag `--skip-database-checks` is **REQUIRED** when running the validator for cna and assay information files because you would need access to internal bed and clinical database tables respectively without it. Without the flag, you will hit an Synapse `READ access` error.
+
+
+#### Examples
+
+Running validator on cna file.
+
+```
+genie validate data_cna_SAGE.txt SAGE --skip-database-checks
+```
+
+Running validator on assay_information file.
+
+```
+genie validate assay_information.yaml SAGE --skip-database-checks
+```

genie/__main__.py

@@ -95,10 +95,9 @@ def build_parser():
     )
 
     parser_validate.add_argument(
-        "--nosymbol-check",
+        "--skip-database-checks",
         action="store_true",
-        help="Ignores specific post-processing validation criteria related to HUGO symbols "
-        "in the structural variant and cna files.",
+        help="Ignores validation checks that require internal database access",
     )
 
     # TODO: remove this default when private genie project is ready

genie/input_to_database.py

@@ -349,7 +349,7 @@ def validatefile(
         #    validator.entitylist = [syn.get(entity) for entity in entities]
 
         valid_cls, message = validator.validate_single_file(
-            oncotree_link=genie_config["oncotreeLink"], nosymbol_check=False
+            oncotree_link=genie_config["oncotreeLink"], skip_database_checks=False
         )
 
         logger.info("VALIDATION COMPLETE")

genie/validate.py

@@ -132,7 +132,7 @@ def validate_single_file(self, **kwargs):
 class GenieValidationHelper(ValidationHelper):
     """A validator helper class for AACR Project Genie."""
 
-    _validate_kwargs = ["nosymbol_check"]
+    _validate_kwargs = ["skip_database_checks"]
 
 
 # TODO: Currently only checks if a user has READ permissions
@@ -249,7 +249,7 @@ def _perform_validate(syn, args):
         genie_config=genie_config,
     )
     mykwargs = dict(
-        nosymbol_check=args.nosymbol_check,
+        skip_database_checks=args.skip_database_checks,
         project_id=args.project_id,
     )
     valid, message = validator.validate_single_file(**mykwargs)

genie_registry/assay.py

@@ -1,6 +1,7 @@
 """Assay information class"""
 
 import os
+from typing import Tuple
 
 import pandas as pd
 import yaml
@@ -15,7 +16,7 @@ class Assayinfo(FileTypeFormat):
 
     _process_kwargs = ["newPath", "databaseSynId"]
 
-    # _validation_kwargs = ["project_id"]
+    _validation_kwargs = ["skip_database_checks"]  # project_id
 
     def _validateFilename(self, filepath_list):
         """Validate assay information filename"""
@@ -127,15 +128,19 @@ def _get_dataframe(self, filepath_list):
             all_panel_info = pd.concat([all_panel_info, assay_finaldf])
         return all_panel_info
 
-    def _validate(self, assay_info_df):
+    def _validate(
+        self, assay_info_df: pd.DataFrame, skip_database_checks: bool
+    ) -> Tuple[str, str]:
         """
         Validates the values of assay information file
 
         Args:
-            assay_info_df: assay information dataframe
+            assay_info_df (pd.DataFrame): input assay information dataframe
+            skip_database_checks (bool): Whether to skip certain validation checks
+                since they requires access to the internal database tables
 
         Returns:
-            tuple: error and warning
+            Tuple[str, str]: complete error and warning messages
         """
 
         total_error = ""
@@ -153,25 +158,9 @@ def _validate(self, assay_info_df):
                     "SEQ_ASSAY_IDs start with your center abbreviation.\n"
                 )
 
-            uniq_seq_df = extract.get_syntabledf(
-                self.syn,
-                f"select distinct(SEQ_ASSAY_ID) as seq from {self.genie_config['sample']} "
-                f"where CENTER = '{self.center}'",
+            total_error += self.validate_all_seq_assay_ids_exist_in_clinical_database(
+                all_seq_assays=all_seq_assays, skip_database_checks=skip_database_checks
             )
-            # These are all the SEQ_ASSAY_IDs that are in the clinical database
-            # but not in the assay_information file
-            missing_seqs = uniq_seq_df["seq"][
-                ~uniq_seq_df["seq"]
-                .replace({"_": "-"}, regex=True)
-                .str.upper()
-                .isin(all_seq_assays)
-            ]
-            missing_seqs_str = ", ".join(missing_seqs)
-            if missing_seqs.to_list():
-                total_error += (
-                    "Assay_information.yaml: You are missing SEQ_ASSAY_IDs: "
-                    f"{missing_seqs_str}\n"
-                )
 
         else:
             total_error += "Assay_information.yaml: Must have SEQ_ASSAY_ID column.\n"
@@ -390,3 +379,49 @@ def _validate(self, assay_info_df):
         total_error += error
 
         return total_error, warning
+
+    def validate_all_seq_assay_ids_exist_in_clinical_database(
+        self, all_seq_assays: dict, skip_database_checks: bool
+    ) -> str:
+        """Validates that all SEQ_ASSAY_IDs in the clinical sample database
+            for that center exists in the assay information file for that center.
+
+        **Conditions**
+
+        | Condition | Result |
+        |---|:---:|
+        | Assay information file has more SEQ_ASSAY_IDs than in clinical database | ✅ PASS |
+        | Assay information file has the same SEQ_ASSAY_IDs as in clinical database | ✅ PASS |
+        | Assay information file has less SEQ_ASSAY_IDs than in clinical database | ❌ FAIL |
+
+        Args:
+            all_seq_assays (dict): list of all the SEQ_ASSAY_IDs in
+                the assay information file
+            skip_database_checks (bool): Whether to skip this validation check
+                since it requires access to the internal clinical sample database
+
+        Returns:
+            str: error message
+        """
+        error = ""
+        if not skip_database_checks:
+            uniq_seq_df = extract.get_syntabledf(
+                self.syn,
+                f"select distinct(SEQ_ASSAY_ID) as seq from {self.genie_config['sample']} "
+                f"where CENTER = '{self.center}'",
+            )
+            # These are all the SEQ_ASSAY_IDs that are in the clinical database
+            # but not in the assay_information file
+            missing_seqs = uniq_seq_df["seq"][
+                ~uniq_seq_df["seq"]
+                .replace({"_": "-"}, regex=True)
+                .str.upper()
+                .isin(all_seq_assays)
+            ]
+            missing_seqs_str = ", ".join(missing_seqs)
+            if missing_seqs.to_list():
+                error += (
+                    "Assay_information.yaml: You are missing SEQ_ASSAY_IDs: "
+                    f"{missing_seqs_str}\n"
+                )
+        return error

genie_registry/cna.py

@@ -1,6 +1,6 @@
 import logging
 import os
-from typing import Union
+from typing import Tuple, Union
 
 import pandas as pd
 import synapseclient
@@ -114,7 +114,7 @@ class cna(FileTypeFormat):
 
     _process_kwargs = ["newPath"]
 
-    _validation_kwargs = ["nosymbol_check"]
+    _validation_kwargs = ["skip_database_checks"]
 
     # VALIDATE FILENAME
     def _validateFilename(self, filePath):
@@ -175,7 +175,18 @@ def process_steps(self, cnaDf, newPath):
             self.syn.store(synapseclient.File(newPath, parent=centerMafSynId))
         return newPath
 
-    def _validate(self, cnvDF, nosymbol_check):
+    def _validate(self, cnvDF: pd.DataFrame, skip_database_checks: bool) -> Tuple:
+        """
+        Validates the values of the input cna file
+
+        Args:
+            cnvDF (pd.DataFrame): input CNA file
+            skip_database_checks (bool): Whether to skip this validation check
+                since it requires access to the internal clinical sample database
+
+        Returns:
+            Tuple: complete error and warning messages
+        """
         total_error = ""
         warning = ""
         cnvDF.columns = [col.upper() for col in cnvDF.columns]
@@ -220,27 +231,49 @@ def _validate(self, cnvDF, nosymbol_check):
             )
         else:
             cnvDF["HUGO_SYMBOL"] = keepSymbols
-            if haveColumn and not nosymbol_check:
-                bedSynId = self.genie_config["bed"]
-                bed = self.syn.tableQuery(
-                    f"select Hugo_Symbol, ID from {bedSynId} "
-                    f"where CENTER = '{self.center}'"
-                )
-                bedDf = bed.asDataFrame()
-                cnvDF["remapped"] = cnvDF["HUGO_SYMBOL"].apply(
-                    lambda x: validateSymbol(x, bedDf)
+            if haveColumn:
+                total_error += self.validate_no_dup_symbols_after_remapping(
+                    cnvDF=cnvDF, skip_database_checks=skip_database_checks
                 )
-                cnvDF = cnvDF[~cnvDF["remapped"].isnull()]
-
-                # Do not allow any duplicated genes after symbols
-                # have been remapped
-                if sum(cnvDF["remapped"].duplicated()) > 0:
-                    duplicated = cnvDF["remapped"].duplicated(keep=False)
-                    total_error += (
-                        "Your CNA file has duplicated Hugo_Symbols "
-                        "(After remapping of genes): {} -> {}.\n".format(
-                            ",".join(cnvDF["HUGO_SYMBOL"][duplicated]),
-                            ",".join(cnvDF["remapped"][duplicated]),
-                        )
-                    )
         return (total_error, warning)
+
+    def validate_no_dup_symbols_after_remapping(
+        self, cnvDF: pd.DataFrame, skip_database_checks: bool
+    ) -> str:
+        """Validates that there are no duplicated Hugo_Symbol values
+            after remapping the previous Hugo_Symbol column using the
+            bed database table. See validateSymbol for more details
+            on the remapping method.
+
+        Args:
+            skip_database_checks (bool): Whether to skip this validation check
+                since it requires access to the internal bed database
+
+        Returns:
+            str: error message
+        """
+        error = ""
+        if not skip_database_checks:
+            bedSynId = self.genie_config["bed"]
+            bed = self.syn.tableQuery(
+                f"select Hugo_Symbol, ID from {bedSynId} "
+                f"where CENTER = '{self.center}'"
+            )
+            bedDf = bed.asDataFrame()
+            cnvDF["remapped"] = cnvDF["HUGO_SYMBOL"].apply(
+                lambda x: validateSymbol(x, bedDf)
+            )
+            cnvDF = cnvDF[~cnvDF["remapped"].isnull()]
+
+            # Do not allow any duplicated genes after symbols
+            # have been remapped
+            if sum(cnvDF["remapped"].duplicated()) > 0:
+                duplicated = cnvDF["remapped"].duplicated(keep=False)
+                error += (
+                    "Your CNA file has duplicated Hugo_Symbols "
+                    "(After remapping of genes): {} -> {}.\n".format(
+                        ",".join(cnvDF["HUGO_SYMBOL"][duplicated]),
+                        ",".join(cnvDF["remapped"][duplicated]),
+                    )
+                )
+        return error