Merge pull request #79 from broadinstitute/jlc_regularize_stored_ontologyID

Regularize stored ontology IDs (SCP-2116)

Author: jlchang

ingest/ingest_pipeline.py

@@ -227,8 +227,13 @@ def load(
                 self.insert_many('data_arrays', documents)
         except Exception as e:
             self.error_logger.error(e, extra=self.extra_log_params)
-            if e.details is not None:
+            if hasattr(e, 'details') and e.details is not None:
                 self.error_logger.error(e.details, extra=self.extra_log_params)
+            else:
+                self.error_logger.error(
+                    'Error loading data to MongoDB (no details available) - check MongoDB access',
+                    extra=self.extra_log_params,
+                )
             return 1
         return 0
 

ingest/validation/validate_metadata.py

@@ -30,6 +30,7 @@
 from colorama import Fore
 import jsonschema
 from google.cloud import bigquery
+import numpy as np
 
 sys.path.append('..')
 try:
@@ -292,10 +293,24 @@ def cast_boolean_type(value):
         raise ValueError(f'cannot cast {value} as boolean')
 
 
+def value_is_nan(value):
+    """Check if value is nan
+    nan is a special dataframe value to indicate missing data
+    """
+    try:
+        return np.isnan(value)
+    except TypeError:
+        return False
+
+
 def cast_integer_type(value):
     """Cast metadata value as integer
     """
-    return int(value)
+    if value_is_nan(value):
+        # nan indicates missing data, has no valid integer value for casting
+        return value
+    else:
+        return int(value)
 
 
 def cast_float_type(value):
@@ -307,12 +322,27 @@ def cast_float_type(value):
 def cast_string_type(value):
     """Cast string type per convention where Pandas autodetected a number
     """
-    if isinstance(value, numbers.Number):
+    if value_is_nan(value):
+        # nan indicates missing data; by type, nan is a numpy float
+        # so a separate type check is needed for proper handling
+        return value
+    elif isinstance(value, numbers.Number):
         return str(value)
     else:
         return value
 
 
+def regularize_ontology_id(value):
+    """Regularize ontology_ids for storage with underscore format
+    """
+    try:
+        return value.replace(":", "_")
+    except AttributeError:
+        # when expected value is not actually an ontology ID
+        # return the bad value for JSON schema validation
+        return value
+
+
 def cast_metadata_type(metadatum, value, id_for_error_detail, convention, metadata):
     """for metadatum, lookup expected type by metadata convention
         and cast value as appropriate type for validation
@@ -332,6 +362,12 @@ def cast_metadata_type(metadatum, value, id_for_error_detail, convention, metada
             # files that support array-based metadata navtively (eg. loom,
             # anndata etc) splitting on pipe may become problematic
             for element in value.split('|'):
+                try:
+                    if 'ontology' in convention['properties'][metadatum]:
+                        element = regularize_ontology_id(element)
+                except KeyError:
+                    # unconventional metadata will trigger this exception
+                    pass
                 cast_element = metadata_types.get(
                     lookup_metadata_type(convention, metadatum)
                 )(element)
@@ -349,8 +385,22 @@ def cast_metadata_type(metadatum, value, id_for_error_detail, convention, metada
         # metadata is being cast - the value needs to be passed as an array,
         # it is already boolean via Pandas' inference processes
         except AttributeError:
-            cast_metadata[metadatum] = [value]
+            try:
+                if 'ontology' in convention['properties'][metadatum]:
+                    value = regularize_ontology_id(value)
+            except KeyError:
+                # unconventional metadata will trigger this exception
+                pass
+            cast_metadata[metadatum] = [
+                metadata_types.get(lookup_metadata_type(convention, metadatum))(value)
+            ]
     else:
+        try:
+            if 'ontology' in convention['properties'][metadatum]:
+                value = regularize_ontology_id(value)
+        except KeyError:
+            # unconventional metadata will trigger this exception
+            pass
         try:
             cast_value = metadata_types.get(
                 lookup_metadata_type(convention, metadatum)
@@ -555,9 +605,9 @@ def retrieve_ontology_term(convention_url, ontology_id, ontologies):
     # valid separators are underscore and colon (used by HCA)
     try:
         ontology_shortname, term_id = re.split('[_:]', ontology_id)
-    # when ontolgyID is malformed and has no separator -> ValueError
-    # when ontologyID value is empty string -> TypeError
     except (ValueError, TypeError):
+        # when ontology_id is malformed and has no separator -> ValueError
+        # when ontology_id value is empty string -> TypeError
         return None
     # check if we have already retrieved this ontology reference
     if ontology_shortname not in ontologies:

tests/data/issues_metadata_v1.1.3.json

@@ -20,7 +20,10 @@
         "BM01_16dpp_CGGTAAACCATT",
         "BM01_16dpp_AAGCAGTGGTAT"
       ],
-      "is_living: 'nan' is not one of ['yes', 'no', 'unknown']": [
+      "is_living: nan is not one of ['yes', 'no', 'unknown']": [
+        "BM01_16dpp_TAAGCAGTGGTA"
+      ],
+      "is_living: nan is not of type 'string'": [
         "BM01_16dpp_TAAGCAGTGGTA"
       ],
       "sample_type: 'direct from donr - fresh' is not one of ['cell line', 'organoid', 'direct from donor - fresh', 'direct from donor - frozen', 'cultured primary cells']": [

tests/data/issues_ontology_v1.1.3.json

@@ -1,10 +1,13 @@
 {
   "error": {
     "convention": {
-      "cell_type: 'nan' does not match '^[A-Za-z]+[_:][0-9]'": [
+      "cell_type: nan is not of type 'string'": [
         "BM01_16dpp_AAGCAGTGGTAT"
       ],
-      "geographical_region: 'nan' does not match '^[A-Za-z]+[_:][0-9]'": [
+      "geographical_region: nan is not of type 'string'": [
+        "BM01_16dpp_AAGCAGTGGTAT"
+      ],
+      "geographical_region__ontology_label: nan is not of type 'string'": [
         "BM01_16dpp_AAGCAGTGGTAT"
       ],
       "library_preparation_protocol: 'EFO0008919' does not match '^[A-Za-z]+[_:][0-9]'": [