From 81c8ce4219788b5036d27be73f7a96b107449c54 Mon Sep 17 00:00:00 2001 From: george-hall-ucl Date: Wed, 5 Jul 2023 12:41:47 +0100 Subject: [PATCH 1/5] #81 Add support for gene sets This adds the flag `GATEWAY_ENABLE_GENE_SETS` to enable support for gene sets. To simplify implementation, activating this flag also activates `GATEWAY_ENABLE_ANNOTATIONS`. The gene sets are saved in a file that has the same name as the annotations `csv` but with `_gene_sets` appended to the file name (before the extension). This file is hidden in filecrawler, and the gene sets are loaded when the associated annotations file is loaded. If the annotations file is missing, then an Exception is raised. I have updated one unit test to make it expect `--disable-gene-sets-save` in the default case (i.e. if `GATEWAY_ENABLE_ANNOTATIONS = 0`). All units tests pass. I have updated the README to document `GATEWAY_ENABLE_GENE_SETS`. --- README.md | 1 + cellxgene_gateway/env.py | 7 +++++++ cellxgene_gateway/filecrawl.py | 12 ++++++++++-- cellxgene_gateway/subprocess_backend.py | 17 ++++++++++++++++- tests/test_subprocess_backend.py | 2 +- 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 3200042..eed75f6 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ Optional environment variables: * `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. +* `GATEWAY_ENABLE_GENE_SETS` - Set to `true` or to `1` to enable cellxgene gene sets. Also enables `GATEWAY_ENABLE_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. diff --git a/cellxgene_gateway/env.py b/cellxgene_gateway/env.py index 5cf0c1b..4d8c4db 100644 --- a/cellxgene_gateway/env.py +++ b/cellxgene_gateway/env.py @@ -26,10 +26,16 @@ extra_scripts = os.environ.get("GATEWAY_EXTRA_SCRIPTS") expire_seconds = int( os.environ.get("GATEWAY_EXPIRE_SECONDS", os.environ.get("GATEWAY_TTL", "3600")) ) +enable_gene_sets = os.environ.get("GATEWAY_ENABLE_GENE_SETS", "").lower() in [ + "true", + "1", +] enable_annotations = os.environ.get("GATEWAY_ENABLE_ANNOTATIONS", "").lower() in [ "true", "1", ] +# Enable annotations if gene sets are enabled: +enable_annotations = enable_annotations or enable_gene_sets enable_backed_mode = os.environ.get("GATEWAY_ENABLE_BACKED_MODE", "").lower() in [ "true", "1", @@ -54,6 +60,7 @@ optional_env_vars = { "GATEWAY_EXTRA_SCRIPTS": extra_scripts, "GATEWAY_EXPIRE_SECONDS": expire_seconds, "GATEWAY_ENABLE_ANNOTATIONS": enable_annotations, + "GATEWAY_ENABLE_GENE_SETS": enable_gene_sets, "GATEWAY_ENABLE_BACKED_MODE": enable_backed_mode, "GATEWAY_LOG_LEVEL": log_level, "CELLXGENE_ARGS": cellxgene_args, diff --git a/cellxgene_gateway/filecrawl.py b/cellxgene_gateway/filecrawl.py index f812091..582afe8 100644 --- a/cellxgene_gateway/filecrawl.py +++ b/cellxgene_gateway/filecrawl.py @@ -20,15 +20,23 @@ def render_annotations(item, item_source): item_source.get_annotations_subpath(item), item_source.name ) new_annotation = f"new" + non_gene_set_files = [] + if item.annotations is not None: + # Do not also display files to store gene_sets. These should be loaded + # by clicking on the associated annotations file (i.e. without the + # appended "_gene_sets") + for a in item.annotations: + if (len(a.name) < 10) or (a.name[-10:] != "_gene_sets"): + non_gene_set_files.append(a) annotations = ( ", ".join( [ f"{a.name}" - for a in item.annotations + for a in non_gene_set_files ] ) + ", " - if item.annotations + if non_gene_set_files else "" ) return " | annotations: " + annotations + new_annotation diff --git a/cellxgene_gateway/subprocess_backend.py b/cellxgene_gateway/subprocess_backend.py index 19f8dc9..8825875 100644 --- a/cellxgene_gateway/subprocess_backend.py +++ b/cellxgene_gateway/subprocess_backend.py @@ -14,7 +14,12 @@ 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, + enable_gene_sets, +) from cellxgene_gateway.process_exception import ProcessException logger = logging.getLogger(__name__) @@ -32,6 +37,16 @@ class SubprocessBackend: extra_args = f" --annotations-file {annotation_file_path}" else: extra_args = " --disable-annotations" + if enable_gene_sets and not annotation_file_path is None: + if annotation_file_path == "": + raise Exception( + "GATEWAY_ENABLE_GENE_SETS is true but --annotation_file_path not set" + ) + else: + 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-gene-sets-save" if enable_backed_mode: extra_args += " --backed" if not cellxgene_args is None: diff --git a/tests/test_subprocess_backend.py b/tests/test_subprocess_backend.py index 5c22dbf..62652bf 100644 --- a/tests/test_subprocess_backend.py +++ b/tests/test_subprocess_backend.py @@ -33,7 +33,7 @@ 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 --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 --disable-gene-sets-save --scripts http://example.com/script.js --scripts http://example.com/script2.js" ], shell=True, stderr=-1, From 5a650334dfc5fd4e72e78120e508a1d9acc37c80 Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Thu, 6 Jul 2023 07:56:43 -0600 Subject: [PATCH 2/5] #81 moved gene set check into fileitem_source --- Changelog.md | 4 ++++ cellxgene_gateway/filecrawl.py | 12 ++---------- cellxgene_gateway/items/file/fileitem_source.py | 3 +++ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Changelog.md b/Changelog.md index 2a93964..b8d8239 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +# 0.3.11 + +* #81 added support for gene sets via GATEWAY_ENABLE_GENE_SETS + # 0.3.10 * #65 Added GATEWAY_EXPIRE_SECONDS to set how long cellxgene servers can remain idle before being terminated. diff --git a/cellxgene_gateway/filecrawl.py b/cellxgene_gateway/filecrawl.py index 582afe8..f812091 100644 --- a/cellxgene_gateway/filecrawl.py +++ b/cellxgene_gateway/filecrawl.py @@ -20,23 +20,15 @@ def render_annotations(item, item_source): item_source.get_annotations_subpath(item), item_source.name ) new_annotation = f"new" - non_gene_set_files = [] - if item.annotations is not None: - # Do not also display files to store gene_sets. These should be loaded - # by clicking on the associated annotations file (i.e. without the - # appended "_gene_sets") - for a in item.annotations: - if (len(a.name) < 10) or (a.name[-10:] != "_gene_sets"): - non_gene_set_files.append(a) annotations = ( ", ".join( [ f"{a.name}" - for a in non_gene_set_files + for a in item.annotations ] ) + ", " - if non_gene_set_files + if item.annotations else "" ) return " | annotations: " + annotations + new_annotation diff --git a/cellxgene_gateway/items/file/fileitem_source.py b/cellxgene_gateway/items/file/fileitem_source.py index 4ade4b9..38d0a8f 100644 --- a/cellxgene_gateway/items/file/fileitem_source.py +++ b/cellxgene_gateway/items/file/fileitem_source.py @@ -35,6 +35,8 @@ 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) @@ -180,6 +182,7 @@ 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: From 2754bc1ef1abe98ef9e41237fd170a0e50e8a5f7 Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Thu, 6 Jul 2023 08:45:05 -0600 Subject: [PATCH 3/5] #81 Combined GATEWAY_ENABLE_ANNOTATIONS and GATEWAY_ENABLE_GENE_SETS flags --- Changelog.md | 2 +- README.md | 3 +-- cellxgene_gateway/cache_entry.py | 1 - cellxgene_gateway/env.py | 7 ------- cellxgene_gateway/gateway.py | 2 -- cellxgene_gateway/items/file/fileitem_source.py | 7 +++++-- cellxgene_gateway/subprocess_backend.py | 10 +--------- 7 files changed, 8 insertions(+), 24 deletions(-) diff --git a/Changelog.md b/Changelog.md index b8d8239..10aad9b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,6 @@ # 0.3.11 -* #81 added support for gene sets via GATEWAY_ENABLE_GENE_SETS +* #81 added support for gene sets # 0.3.10 diff --git a/README.md b/README.md index eed75f6..13ba916 100644 --- a/README.md +++ b/README.md @@ -75,8 +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. -* `GATEWAY_ENABLE_GENE_SETS` - Set to `true` or to `1` to enable cellxgene gene sets. Also enables `GATEWAY_ENABLE_ANNOTATIONS`. +* `GATEWAY_ENABLE_ANNOTATIONS` - Set to `true` or to `1` to enable cellxgene annotations and gene sets. * `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. diff --git a/cellxgene_gateway/cache_entry.py b/cellxgene_gateway/cache_entry.py index aa3bb0c..8c72efc 100644 --- a/cellxgene_gateway/cache_entry.py +++ b/cellxgene_gateway/cache_entry.py @@ -58,7 +58,6 @@ class CacheEntry: @classmethod def for_key(cls, key, port): - return cls( None, key, diff --git a/cellxgene_gateway/env.py b/cellxgene_gateway/env.py index 4d8c4db..5cf0c1b 100644 --- a/cellxgene_gateway/env.py +++ b/cellxgene_gateway/env.py @@ -26,16 +26,10 @@ extra_scripts = os.environ.get("GATEWAY_EXTRA_SCRIPTS") expire_seconds = int( os.environ.get("GATEWAY_EXPIRE_SECONDS", os.environ.get("GATEWAY_TTL", "3600")) ) -enable_gene_sets = os.environ.get("GATEWAY_ENABLE_GENE_SETS", "").lower() in [ - "true", - "1", -] enable_annotations = os.environ.get("GATEWAY_ENABLE_ANNOTATIONS", "").lower() in [ "true", "1", ] -# Enable annotations if gene sets are enabled: -enable_annotations = enable_annotations or enable_gene_sets enable_backed_mode = os.environ.get("GATEWAY_ENABLE_BACKED_MODE", "").lower() in [ "true", "1", @@ -60,7 +54,6 @@ optional_env_vars = { "GATEWAY_EXTRA_SCRIPTS": extra_scripts, "GATEWAY_EXPIRE_SECONDS": expire_seconds, "GATEWAY_ENABLE_ANNOTATIONS": enable_annotations, - "GATEWAY_ENABLE_GENE_SETS": enable_gene_sets, "GATEWAY_ENABLE_BACKED_MODE": enable_backed_mode, "GATEWAY_LOG_LEVEL": log_level, "CELLXGENE_ARGS": cellxgene_args, diff --git a/cellxgene_gateway/gateway.py b/cellxgene_gateway/gateway.py index c930974..1b9c14e 100644 --- a/cellxgene_gateway/gateway.py +++ b/cellxgene_gateway/gateway.py @@ -80,7 +80,6 @@ cache = BackendCache() @app.errorhandler(CellxgeneException) def handle_invalid_usage(error): - message = f"{error.http_status} Error : {error.message}" return ( @@ -95,7 +94,6 @@ def handle_invalid_usage(error): @app.errorhandler(ProcessException) def handle_invalid_process(error): - message = [] message.append(error.message) diff --git a/cellxgene_gateway/items/file/fileitem_source.py b/cellxgene_gateway/items/file/fileitem_source.py index 38d0a8f..407f3a8 100644 --- a/cellxgene_gateway/items/file/fileitem_source.py +++ b/cellxgene_gateway/items/file/fileitem_source.py @@ -35,8 +35,11 @@ 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_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) diff --git a/cellxgene_gateway/subprocess_backend.py b/cellxgene_gateway/subprocess_backend.py index 8825875..0dc87be 100644 --- a/cellxgene_gateway/subprocess_backend.py +++ b/cellxgene_gateway/subprocess_backend.py @@ -18,7 +18,6 @@ from cellxgene_gateway.env import ( cellxgene_args, enable_annotations, enable_backed_mode, - enable_gene_sets, ) from cellxgene_gateway.process_exception import ProcessException @@ -35,17 +34,10 @@ class SubprocessBackend: extra_args = f" --annotations-dir {make_annotations(file_path)}" else: extra_args = f" --annotations-file {annotation_file_path}" - else: - extra_args = " --disable-annotations" - if enable_gene_sets and not annotation_file_path is None: - if annotation_file_path == "": - raise Exception( - "GATEWAY_ENABLE_GENE_SETS is true but --annotation_file_path not set" - ) - else: 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" From 7b314d4457b421755fad20cc8f3b57cb85784f26 Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Thu, 6 Jul 2023 17:15:00 -0600 Subject: [PATCH 4/5] #81 switch to latest ubuntu --- .github/workflows/pr-checks.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 931d8f4..17d6cd0 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -6,7 +6,7 @@ on: [push, pull_request] jobs: black: - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest 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-18.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 From 64d636a1c57b61086e1f9fb493566a17547dec51 Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Sun, 9 Jul 2023 07:37:06 -0400 Subject: [PATCH 5/5] #81 added unit test for gene sets --- tests/test_subprocess_backend.py | 37 +++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/test_subprocess_backend.py b/tests/test_subprocess_backend.py index 62652bf..f4f839e 100644 --- a/tests/test_subprocess_backend.py +++ b/tests/test_subprocess_backend.py @@ -1,7 +1,6 @@ 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 @@ -40,3 +39,39 @@ class TestSubprocessBackend(unittest.TestCase): 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, + )