#73 moved new annotation link to front

.github/workflows/pr-checks.yaml

@@ -6,7 +6,7 @@ on: [push, pull_request]
 
 jobs:
   black:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-18.04
     steps:
       - uses: actions/checkout@v2
         name: Checkout repository
@@ -25,7 +25,7 @@ jobs:
           black . --check
   # This job is copied over from `deploy.yaml`
   run-tests:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-18.04
     steps:
       - uses: actions/checkout@v2
 

Changelog.md

@@ -1,9 +1,3 @@
-# 0.3.11
-
-* #81 added support for gene sets
-* #79 added example for cellxgene-gateway customized docker image
-* #78 prune directories that do not contain h5ad files
-
 # 0.3.10
 
 * #65 Added GATEWAY_EXPIRE_SECONDS to set how long cellxgene servers can remain idle before being terminated.

README.md

@@ -75,7 +75,7 @@ Optional environment variables:
 * `GATEWAY_PORT` - local port that the gateway should bind to, defaults to 5005
 * `GATEWAY_EXPIRE_SECONDS` - time in seconds that a cellxgene process will remain idle before being terminated. Defaults to 3600 (one hour)
 * `GATEWAY_EXTRA_SCRIPTS` - JSON array of script paths, will be embedded into each page and forwarded with `--scripts` to cellxgene server
-* `GATEWAY_ENABLE_ANNOTATIONS` - Set to `true` or to `1` to enable cellxgene annotations and gene sets.
+* `GATEWAY_ENABLE_ANNOTATIONS` - Set to `true` or to `1` to enable cellxgene annotations. 
 * `GATEWAY_ENABLE_BACKED_MODE` - Set to `true` or to `1` to load AnnData in file-backed mode. This saves memory and speeds up launch time but may reduce overall performance.
 * `GATEWAY_LOG_LEVEL` - default is `INFO`. set to `DEBUG` to increase logging and to `WARNING` to decrease logging.
 * `S3_ENABLE_LISTINGS_CACHE` - Set to `true` or to `1` to cache listings of S3 folders for performance. If the cache becomes stale, set `filecrawl.html?refresh=true` query parameter to refresh the cache.

cellxgene_gateway/__init__.py

@@ -7,4 +7,4 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-__version__ = "0.3.11"
+__version__ = "0.3.10"

cellxgene_gateway/cache_entry.py

@@ -58,6 +58,7 @@ class CacheEntry:
 
     @classmethod
     def for_key(cls, key, port):
+
         return cls(
             None,
             key,

cellxgene_gateway/filecrawl.py

@@ -7,7 +7,7 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-import os
+import html
 import urllib.parse
 
 from cellxgene_gateway import env, flask_util
@@ -19,19 +19,17 @@ def render_annotations(item, item_source):
     url = flask_util.view_url(
         item_source.get_annotations_subpath(item), item_source.name
     )
-    new_annotation = f"<a class='new' href='{url}'>new</a>"
+    new_annotation = [f"<a class='new' href='{url}'>new</a>"]
+
     annotations = (
-        ", ".join(
-            [
-                f"<a href='{CacheKey(item, item_source, a).view_url}/'>{a.name}</a>"
-                for a in item.annotations
-            ]
-        )
-        + ", "
+        [
+            f"<a href='{CacheKey(item, item_source, a).view_url}/'>{html.escape(a.name)}</a>"
+            for a in item.annotations
+        ]
         if item.annotations
-        else ""
+        else []
     )
-    return " | annotations: " + annotations + new_annotation
+    return "| annotations: " + ", ".join(new_annotation + annotations)
 
 
 def render_item(item, item_source):

cellxgene_gateway/gateway.py

@@ -80,6 +80,7 @@ cache = BackendCache()
 
 @app.errorhandler(CellxgeneException)
 def handle_invalid_usage(error):
+
     message = f"{error.http_status} Error : {error.message}"
 
     return (
@@ -94,6 +95,7 @@ def handle_invalid_usage(error):
 
 @app.errorhandler(ProcessException)
 def handle_invalid_process(error):
+
     message = []
 
     message.append(error.message)

cellxgene_gateway/items/file/fileitem_source.py

@@ -35,11 +35,6 @@ class FileItemSource(ItemSource):
     def name(self):
         return self._name or f"Files:{self.base_path}"
 
-    def is_gene_set(self, path: str) -> bool:
-        return ("_gene_sets" in path or "-gene-sets" in path) and path.endswith(
-            self.annotation_file_suffix
-        )
-
     def is_h5ad_file(self, path: str) -> bool:
         return path.endswith(self.h5ad_suffix) and os.path.isfile(path)
 
@@ -68,7 +63,7 @@ class FileItemSource(ItemSource):
 
         return item_tree
 
-    def scan_directory(self, subpath: str = "") -> ItemTree:
+    def scan_directory(self, subpath="") -> dict:
         base_path = os.path.join(self.base_path, subpath)
 
         if not os.path.exists(base_path):
@@ -105,11 +100,6 @@ class FileItemSource(ItemSource):
             branches = [
                 self.scan_directory(os.path.join(subpath, subdir)) for subdir in subdirs
             ]
-            # Exclude branches without files as leaves. Since traversal is applied pre-order,
-            # branch.branches has already been processed and we don't need to check deeper nesting.
-            branches = [
-                branch for branch in branches if branch.items or branch.branches
-            ]
 
         return ItemTree(subpath, items, branches)
 
@@ -190,7 +180,6 @@ class FileItemSource(ItemSource):
                 self.make_fileitem_from_path(annotation, annotations_subpath, True)
                 for annotation in sorted(os.listdir(annotations_fullpath))
                 if annotation.endswith(self.annotation_file_suffix)
-                and not self.is_gene_set(annotation)
                 and os.path.isfile(os.path.join(annotations_fullpath, annotation))
             ]
         else:

cellxgene_gateway/items/s3/s3item_source.py

@@ -116,9 +116,6 @@ class S3ItemSource(ItemSource):
         branches = None
         if len(subdir_keys) > 0:
             branches = [self.scan_directory(key) for key in subdir_keys]
-            branches = [
-                branch for branch in branches if branch.items or branch.branches
-            ]
 
         return ItemTree(directory_key, items, branches)
 

cellxgene_gateway/subprocess_backend.py

@@ -14,11 +14,7 @@ from flask_api import status
 
 from cellxgene_gateway.cache_entry import CacheEntryStatus
 from cellxgene_gateway.dir_util import make_annotations
-from cellxgene_gateway.env import (
-    cellxgene_args,
-    enable_annotations,
-    enable_backed_mode,
-)
+from cellxgene_gateway.env import cellxgene_args, enable_annotations, enable_backed_mode
 from cellxgene_gateway.process_exception import ProcessException
 
 logger = logging.getLogger(__name__)
@@ -34,11 +30,8 @@ class SubprocessBackend:
                 extra_args = f" --annotations-dir {make_annotations(file_path)}"
             else:
                 extra_args = f" --annotations-file {annotation_file_path}"
-                gene_sets_file_path = annotation_file_path[:-4] + "_gene_sets.csv"
-                extra_args += f" --gene-sets-file {gene_sets_file_path}"
         else:
             extra_args = " --disable-annotations"
-            extra_args += " --disable-gene-sets-save"
         if enable_backed_mode:
             extra_args += " --backed"
         if not cellxgene_args is None:

examples/customized_docker_image/Dockerfile

@@ -1,14 +0,0 @@
-FROM python:3.9
-
-RUN pip install cellxgene-gateway 'MarkupSafe<2.1'
-
-COPY customize_ui.sh customize_ui.sh
-RUN CELLXGENE_GATEWAY_DIR=/usr/local/lib/python3.9/site-packages/cellxgene_gateway . ./customize_ui.sh
-
-ENV CELLXGENE_DATA=/cellxgene-data
-ENV CELLXGENE_LOCATION=/usr/local/bin/cellxgene
-EXPOSE 5005
-
-RUN mkdir /cellxgene-data
-
-CMD ["cellxgene-gateway"]

examples/customized_docker_image/README.md

@@ -1,14 +0,0 @@
-# Purpose
-
-This is a simple example of how to make a small script to customize the UI of cellxgene-gateway. The script that does the customization is `customize_ui.sh`, it simply makes the main header green using CSS but you could do anything you want there (including adding more script tags, etc).
-
-# Usage
-
-```
-docker build -t cellxgene_custom .
-CELLXGENE_DATA=`pwd`/../../../cellxgene_data
-docker run -p 5005:5005 --mount src=$CELLXGENE_DATA,target=/cellxgene-data,type=bind cellxgene_custom
-```
-
-If you now open http://localhost:5005 you should see a green cellxgene gateway header.
-

examples/customized_docker_image/customize_ui.sh

@@ -1,3 +0,0 @@
-# make the header bright green
-find "${CELLXGENE_GATEWAY_DIR}/templates" -name index.html -exec sed -i -e 's/<head>/<head>\
-> <style> header h3 {color: #0F0;} <\/style>/g' {} \;

tests/test_filecrawl.py

@@ -1,5 +1,4 @@
 import unittest
-from collections import defaultdict
 from unittest.mock import MagicMock, patch
 
 from cellxgene_gateway.filecrawl import (
@@ -14,26 +13,76 @@ from cellxgene_gateway.items.item import ItemTree, ItemType
 source = FileItemSource("/tmp")
 
 
+def make_entry(subpath="somepath", annotations=None):
+    return FileItem(
+        subpath=subpath,
+        name="entry",
+        ext=".h5ad",
+        type=ItemType.h5ad,
+        annotations=annotations,
+    )
+
+
 class TestRenderEntry(unittest.TestCase):
     def test_GIVEN_path_both_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="/somepath/", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="/somepath/")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_starts_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="/somepath", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="/somepath")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_ends_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="somepath/", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="somepath/")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_no_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="somepath", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="somepath")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
+
+
+class TestRenderAnnotation(unittest.TestCase):
+    def test_GIVEN_no_annotation_THEN_new_alone(self):
+        entry = make_entry(annotations=None)
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a></li>",
+            rendered,
+        )
+
+    def test_GIVEN_annotation_THEN_new_before(self):
+        annotation = FileItem(
+            subpath="somepath/entry_annotations",
+            name="annot",
+            ext=".csv",
+            type=ItemType.annotation,
+        )
+        entry = make_entry(annotations=[annotation])
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a>,"
+            " <a href='/source/Files:/tmp/view/somepath/entry_annotations/annot.csv/'>annot</a></li>",
+            rendered,
+        )
+
+    def test_GIVEN_annotation_THEN_escaped(self):
+        annotation = FileItem(
+            subpath="somepath/entry_annotations",
+            name="hot&cold",
+            ext=".csv",
+            type=ItemType.annotation,
+        )
+        entry = make_entry(annotations=[annotation])
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a>,"
+            " <a href='/source/Files:/tmp/view/somepath/entry_annotations/hot%26cold.csv/'>hot&amp;cold</a></li>",
+            rendered,
+        )
 
 
 class TestRenderItemSource(unittest.TestCase):
@@ -49,48 +98,12 @@ class TestRenderItemSource(unittest.TestCase):
 
 
 class TestRenderItemTree(unittest.TestCase):
-    def setUp(self):
-        from cellxgene_gateway.gateway import app
-
-        self.app = app
-        self.app_context = self.app.test_request_context()
-        self.app_context.push()
-
     @patch("cellxgene_gateway.items.file.fileitem_source.FileItemSource")
     def test_GIVEN_deep_nested_dirs_THEN_includes_dirs_in_output(self, item_source):
         item_source.name = "FakeSource"
-        item_source.get_annotations_subpath = lambda _: "FakeAnnotations"
-        file_item = FileItem(
-            subpath="foo/bar/baz", name="file.h5ad", type=ItemType.h5ad
-        )
-        item_tree = ItemTree("foo/bar/baz", [file_item], [])
+        item_tree = ItemTree("foo/bar/baz", [], [])
         rendered = render_item_tree(item_tree, item_source)
         self.assertEqual(
             rendered,
-            "<li><a href='/filecrawl/foo/bar/baz?source=FakeSource'>baz</a><ul>"
-            "<li> <a href='/source/FakeSource/view/foo/bar/baz/file.h5ad/'>file.h5ad</a>"
-            "  | annotations: <a class='new' href='/source/FakeSource/view/FakeAnnotations'>new</a>"
-            "</li></ul></li>",
-        )
-
-    @patch(
-        "os.listdir",
-        side_effect=lambda parent: defaultdict(
-            list, {"tmp": ["foo"], "tmp/foo": ["bar"]}
-        )[parent],
-    )
-    @patch("os.path.exists", return_value=True)
-    def test_GIVEN_dirs_without_h5ad_THEN_excludes_dirs_in_output(
-        self, listdir, exists
-    ):
-        # Directories:
-        # - tmp
-        #   - foo
-        #     - bar (no h5ad files)
-        item_source = FileItemSource("tmp", name="local")
-        item_tree = item_source.list_items("foo")
-        rendered = render_item_tree(item_tree, item_source)
-        self.assertEqual(
-            rendered,
-            "<li><a href='/filecrawl/foo?source=local'>foo</a><ul></ul></li>",
+            "<li><a href='/filecrawl/foo/bar/baz?source=FakeSource'>baz</a><ul></ul></li>",
         )

tests/test_subprocess_backend.py

@@ -1,6 +1,7 @@
 import unittest
 from unittest.mock import MagicMock, patch
 
+from cellxgene_gateway.backend_cache import BackendCache
 from cellxgene_gateway.cache_entry import CacheEntry
 from cellxgene_gateway.cache_key import CacheKey
 from cellxgene_gateway.items.file.fileitem import FileItem
@@ -32,46 +33,10 @@ class TestSubprocessBackend(unittest.TestCase):
             backend.launch(cellxgene_loc, scripts, entry)
         popen.assert_called_once_with(
             [
-                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --disable-annotations --disable-gene-sets-save --scripts http://example.com/script.js --scripts http://example.com/script2.js"
+                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --disable-annotations --scripts http://example.com/script.js --scripts http://example.com/script2.js"
             ],
             shell=True,
             stderr=-1,
             stdout=-1,
         )
         self.assertEqual("An unexpected error", context.exception.stderr)
-
-    @patch("subprocess.Popen")
-    def test_launch_GIVEN_annotations_enabled_THEN_set_flags(self, popen):
-        subprocess = MagicMock()
-        subprocess.stdout.readline().decode.return_value = (
-            "[cellxgene] Type CTRL-C at any time to exit.\n"
-        )
-        subprocess.stderr.read().decode.return_value = ""
-        popen.return_value = subprocess
-
-        key = CacheKey(
-            FileItem("/czi/", name="pbmc3k.h5ad", type=ItemType.h5ad),
-            FileItemSource("/tmp", "local"),
-            FileItem(
-                "/czi/pbmc3k_annotations/", name="foo.csv", type=ItemType.annotation
-            ),
-        )
-        entry = CacheEntry.for_key(key, 8000)
-        import cellxgene_gateway.subprocess_backend
-
-        cellxgene_gateway.subprocess_backend.enable_annotations = True
-        try:
-            backend = cellxgene_gateway.subprocess_backend.SubprocessBackend()
-            cellxgene_loc = "/some/cellxgene"
-
-            backend.launch(cellxgene_loc, [], entry)
-        finally:
-            cellxgene_gateway.subprocess_backend.enable_annotations = False
-        popen.assert_called_once_with(
-            [
-                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --annotations-file /tmp/czi/pbmc3k_annotations/foo.csv --gene-sets-file /tmp/czi/pbmc3k_annotations/foo_gene_sets.csv"
-            ],
-            shell=True,
-            stderr=-1,
-            stdout=-1,
-        )