hosted gene sets routes, plus a few bug fixes (#2155)

* first cut at hosted gs routes

* lint

* update tests to match csv parser changes

* update tests to new API

* update gene set name validation rules to match requirements

* add path mapping from dataset to geneset

* add test cases for geneset GET route

* fix test assertion

* remove debugging code

* update gene set uri mapping function

* fix error message

* allow extra user-specified headers in gene set csv file

* clarify comment

backend/server/common/annotations/annotations.py

@@ -5,10 +5,11 @@ import fsspec
 
 from backend.common.errors import OntologyLoadFailure, DisabledFeatureError
 from backend.common.utils.type_conversion_utils import get_schema_type_hint_of_array
+from backend.common.genesets import write_gene_sets_tidycsv
 
 
 class Annotations(metaclass=ABCMeta):
-    """ baseclass for annotations, including ontologies and genesets"""
+    """ baseclass for annotations, including ontologies and gene sets"""
 
     """ our default ontology is the PURL for the Cell Ontology.
     See http://www.obofoundry.org/ontology/cl.html """
@@ -30,7 +31,7 @@ class Annotations(metaclass=ABCMeta):
 
     def check_gene_sets_save_enabled(self):
         if not self.gene_sets_save_enabled():
-            raise DisabledFeatureError("User genesets save is disabled.")
+            raise DisabledFeatureError("User gene sets save is disabled.")
 
     def load_ontology(self, path):
         """Load and parse ontologies - currently support OBO files only."""
@@ -81,12 +82,12 @@ class Annotations(metaclass=ABCMeta):
 
     @abstractmethod
     def read_gene_sets(self, data_adaptor):
-        """Return the genesets from persistent storage """
+        """Return the gene sets from persistent storage """
         pass
 
     @abstractmethod
     def write_gene_sets(self, gs, data_adaptor):
-        """Write the genesets (gs) to a persistent storage such that it can later be read"""
+        """Write the gene sets (gs) to a persistent storage such that it can later be read"""
         pass
 
     @abstractmethod
@@ -94,51 +95,25 @@ class Annotations(metaclass=ABCMeta):
         """Update configuration parameters that describe information about the annotations feature"""
         pass
 
-    Genesets_Header = [
-        "gene_set_name",
-        "gene_set_description",
-        "gene_symbol",
-        "gene_description",
-    ]
-
     @staticmethod
     def gene_sets_to_csv(genesets):
         """
-        Convert the internal genesets format (returned by read_gene_set) into
+        Convert the internal gene sets format (returned by read_gene_set) into
         the simple Tidy CSV.
         """
         from io import StringIO
-        import csv
 
         if isinstance(genesets, dict):
             genesets = genesets.values()
 
         with StringIO() as sio:
-            writer = csv.writer(sio, dialect='excel')
-            writer.writerow(Annotations.Genesets_Header)
-            for geneset in genesets:
-                # genes may be empty, in which case we skip the geneset entirely
-                genes = geneset["genes"]
-                if not genes:
-                    writer.writerow([geneset["geneset_name"], geneset.get("geneset_description", ""), "", ""])
-                else:
-                    writer.writerows(
-                        [
-                            [
-                                geneset["geneset_name"],
-                                geneset.get("geneset_description", ""),
-                                gene["gene_symbol"],
-                                gene.get("gene_description", ""),
-                            ]
-                            for gene in genes
-                        ]
-                    )
+            write_gene_sets_tidycsv(sio, genesets)
             return sio.getvalue()
 
     @staticmethod
     def gene_sets_to_response(genesets):
         """
-        Convert the internal genesets format (returned by read_gene_set) into
+        Convert the internal gene sets format (returned by read_gene_set) into
         the dict expected by the JSON REST API
         """
         return list(genesets.values())

backend/server/common/annotations/local_file_csv.py

@@ -4,14 +4,15 @@ import re
 import threading
 from datetime import datetime
 from hashlib import blake2b
-import csv
 
 import pandas as pd
 from flask import session, has_request_context, current_app
 
 from backend.server import __version__ as cellxgene_version
 from backend.server.common.annotations.annotations import Annotations
+from backend.common.genesets import read_gene_sets_tidycsv
 from backend.common.errors import AnnotationsError, ObsoleteRequest
+from backend.common.utils.data_locator import DataLocator
 
 
 class AnnotationsLocalFile(Annotations):
@@ -124,8 +125,8 @@ class AnnotationsLocalFile(Annotations):
                 if fname == self.last_geneset_fname:
                     gene_sets = self.last_geneset
                 else:
-                    with open(fname, newline="") as f:
-                        gene_sets = read_gene_set_tidycsv(f, context)
+                    # read
+                    gene_sets = read_gene_sets_tidycsv(DataLocator(fname), context)
 
                     # validate
                     gene_sets = data_adaptor.check_new_gene_sets(gene_sets, context)
@@ -259,6 +260,7 @@ class AnnotationsLocalFile(Annotations):
         params = {}
         params["annotations"] = self.user_annotations_enabled()
         params["annotations_genesets_readonly"] = not self.gene_sets_save_enabled()
+        params["annotations_genesets_name_is_read_only"] = self.gene_sets_output_file is not None
         params["user_annotation_collection_name_enabled"] = True
 
         if self.ontology_data:
@@ -276,99 +278,10 @@ class AnnotationsLocalFile(Annotations):
 
         elif session is not None:
             collection = self.get_collection()
-            params["annotations-data-collection-is-read-only"] = False
+            params["annotations-data-collection-is-read-only"] = not self.user_annotations_enabled()
             params["annotations-data-collection-name"] = collection
 
         if current_app.auth.is_user_authenticated():
             params["annotations-user-data-idhash"] = self._get_userdata_idhash(data_adaptor)
 
         parameters.update(params)
-
-
-def read_gene_set_tidycsv(f, context=None):
-    """
-    Read & parse the Tidy CSV format, applying validation checks for mandatory
-    values, and de-duping rules.
-
-    Format is a four-column CSV, with a mandatory header row, and optional "#" prefixed
-    comments.  Format:
-
-        gene_set_name, gene_set_description, gene_symbol, gene_description
-
-    gene_set_name must be non-null; others are optional.
-
-    Returns: a dictionary of the shape (values in angle-brackets vary):
-
-        {
-            <string, a gene set name>: {
-                "geneset_name": <string, a gene set name>,
-                "geneset_description": <a string or None>,
-                "genes": [
-                    {
-                        "gene_symbol": <string, a gene symbol or name>,
-                        "gene_description": <a string or None>
-                    },
-                    ...
-                ]
-            },
-            ...
-        }
-    """
-
-    class myDialect(csv.excel):
-        skipinitialspace = True
-
-    def just(n, seq):
-        it = iter(seq)
-        for _ in range(n - 1):
-            yield next(it, "")
-        yield tuple(it)
-
-    messagefn = context["messagefn"] if context else (lambda x: None)
-
-    reader = csv.reader(f, dialect=myDialect())
-    gene_sets = {}
-    haveReadHeader = False
-    lineno = 0
-    for row in reader:
-        lineno += 1
-        # ignore empty rows
-        if len(row) == 0:
-            continue
-        # if row starts with '#' it is a comment
-        if row[0].startswith("#"):
-            continue
-        # if this is the first non-comment row, assume it is a header
-        if not haveReadHeader:
-            if row != Annotations.Genesets_Header:
-                raise AnnotationsError("Geneset CSV file missing the required column header.")
-            haveReadHeader = True
-            continue
-
-        geneset_name, geneset_description, gene_symbol, gene_description, _ = just(5, row)
-        if not geneset_name:
-            raise AnnotationsError(f"Geneset CSV missing required geneset or gene name on line {lineno}")
-        if (not gene_symbol) and gene_description:
-            messagefn(f"Warning: Missing gene name in geneset name {geneset_name} on line {lineno}.")
-
-        if geneset_name in gene_sets:
-            gs = gene_sets[geneset_name]
-        else:
-            gs = gene_sets[geneset_name] = {
-                "geneset_name": geneset_name,
-                "geneset_description": geneset_description,
-                "genes": [],
-            }
-        # Use first geneset_description with a value
-        if not gs["geneset_description"] and geneset_description:
-            gs["geneset_description"] = geneset_description
-        # add the gene if the gene_symbol is defined
-        if gene_symbol:
-            gs["genes"].append(
-                {
-                    "gene_symbol": gene_symbol,
-                    "gene_description": gene_description,
-                }
-            )
-
-    return gene_sets

backend/server/common/config/dataset_config.py

@@ -138,7 +138,7 @@ class DatasetConfig(BaseConfig):
 
         if dirname is not None and (filename is not None or genesets_filename is not None):
             raise ConfigurationError(
-                "'user-generated-data-dir' may not be used with annotations-file' or 'genesets-file'."
+                "'user-generated-data-dir' may not be used with 'annotations-file' or 'gene-sets-file'."
             )
 
         if filename is not None:

backend/server/common/rest.py

@@ -3,7 +3,7 @@ import logging
 import sys
 from http import HTTPStatus
 import zlib
-import hashlib
+import json
 
 from flask import make_response, jsonify, current_app, abort
 from werkzeug.urls import url_unquote
@@ -22,9 +22,7 @@ from backend.common.errors import (
     ObsoleteRequest,
     UnsupportedSummaryMethod,
 )
-
-import json
-
+from backend.common.genesets import summarizeQueryHash
 from backend.common.fbs.matrix import decode_matrix_fbs
 
 
@@ -390,7 +388,7 @@ def summarize_var_helper(request, data_adaptor, key, raw_query):
         return abort(HTTPStatus.NOT_ACCEPTABLE)
 
     summary_method = request.values.get("method", default="mean")
-    query_hash = hashlib.sha1(raw_query).hexdigest()  # cache helper
+    query_hash = summarizeQueryHash(raw_query)
     if key and query_hash != key:
         return abort(HTTPStatus.BAD_REQUEST, description="query key did not match")
 
@@ -398,7 +396,7 @@ def summarize_var_helper(request, data_adaptor, key, raw_query):
     args_filter_only.poplist("method")
     args_filter_only.poplist("key")
 
-    try:    
+    try:
         filter = _query_parameter_to_filter(args_filter_only)
         return make_response(
             data_adaptor.summarize_var(summary_method, filter, query_hash),

backend/server/data_common/data_adaptor.py

@@ -1,6 +1,5 @@
 from abc import ABCMeta, abstractmethod
 from os.path import basename, splitext
-import re
 import numpy as np
 import pandas as pd
 from scipy import sparse
@@ -11,6 +10,7 @@ from backend.common.constants import Axis
 from backend.common.errors import FilterError, JSONEncodingValueError, ExceedsLimitError, UnsupportedSummaryMethod
 from backend.common.utils.utils import jsonify_numpy
 from backend.common.fbs.matrix import encode_matrix_fbs
+from backend.common.genesets import validate_gene_sets
 
 
 class DataAdaptor(metaclass=ABCMeta):
@@ -263,95 +263,8 @@ class DataAdaptor(metaclass=ABCMeta):
         return labels_df
 
     def check_new_gene_sets(self, genesets, context=None):
-        """
-        Check validity of gene sets, return if correct, else raise error.
-        May also modify the gene set for conditions that should be resolved,
-        but which do not warrant a hard error.
-
-        Argument genesets may be either the REST OTA format (list of dicts) or the internal
-        format (dict of dicts, keyed by the geneset name).
-
-        Will return a modified genesets (eg, remove dups) of the same type as the
-        provided argument. Ie, dict->dict, list->list
-
-        Rules:
-        0. all geneset names must be unique.
-        1. All geneset names must be comprised of legal characters, meaning:
-            * no leading or trailing white space
-            * no multi-space runs
-            * no tab, vertical tab, newline or return
-           Where "space" means ASCII 32. Generates hard error.
-        2. Gene symbols must be part of the current var_index.  If symbol not in var_index,
-           will generate a warning and the symbol removed.
-        3. Duplicate gene symbols are silently de-duped.
-        """
-        messagefn = context["messagefn"] if context else (lambda x: None)
-
-        # accept genesets args as either the internal (dict) or REST (list) format,
-        # as they are identical except for the dict being keyed by geneset_name.
-        if not isinstance(genesets, dict) and not isinstance(genesets, list):
-            raise ValueError("Genesets must be either dict or list.")
-        genesets_iterable = genesets if isinstance(genesets, list) else genesets.values()
-
-        # 0. check for uniqueness of geneset names
-        geneset_names = [gs["geneset_name"] for gs in genesets_iterable]
-        if len(set(geneset_names)) != len(geneset_names):
-            raise KeyError("All geneset names must be unique.")
-
-        # 1. check gene set character set and format
-        illegal_name = re.compile(r"^\s|  |[\v\t\r\n]|\s$")
-        for name in geneset_names:
-            if type(name) != str or len(name) == 0:
-                raise KeyError("Geneset names must be non-null string.")
-            if illegal_name.search(name):
-                messagefn(
-                    "Error: "
-                    f"Geneset name {name} "
-                    "is not valid. Leading, trailing, and multiple spaces within a name are not allowed."
-                )
-                raise KeyError(
-                    "Geneset name is not valid. Leading, trailing, and multiple spaces within a name are not allowed."
-                )
-
-        # 2. & 3. check for duplicate gene symbols, and those not present in the dataset. They will
-        # generate a warning and be removed.
         var_names = set(self.query_var_array(self.parameters.get("var_names")))
-        for geneset in genesets_iterable:
-            if not isinstance(geneset, dict):
-                raise ValueError("Each geneset must be a dict.")
-            geneset_name = geneset["geneset_name"]
-            genes = geneset["genes"]
-            if not isinstance(genes, list):
-                raise ValueError("Geneset genes field must be a list")
-            geneset.setdefault("geneset_description", "")
-            gene_symbol_already_seen = set()
-            new_genes = []
-            for gene in genes:
-                gene_symbol = gene["gene_symbol"]
-                if not isinstance(gene_symbol, str) or len(gene_symbol) == 0:
-                    raise ValueError("Gene symbol must be non-null string.")
-                if gene_symbol in gene_symbol_already_seen:
-                    # duplicate check
-                    messagefn(
-                        f"Warning: a duplicate of gene {gene_symbol} was found in geneset {geneset_name}, "
-                        "and will be ignored."
-                    )
-                    continue
-
-                if gene_symbol not in var_names:
-                    messagefn(
-                        f"Warning: {gene_symbol}, used in geneset {geneset_name}, "
-                        "was not found in the dataset and will be ignored."
-                    )
-                    continue
-
-                gene_symbol_already_seen.add(gene_symbol)
-                gene.setdefault("gene_description", "")
-                new_genes.append(gene)
-
-            geneset["genes"] = new_genes
-
-        return genesets
+        return validate_gene_sets(genesets, var_names)
 
     def data_frame_to_fbs_matrix(self, filter, axis):
         """