From fdd6cca29704a13ff8c9cd17e98971c999e0a91d Mon Sep 17 00:00:00 2001 From: Andreas Eisenbarth Date: Wed, 12 Oct 2022 13:40:39 +0200 Subject: [PATCH 1/6] Rename argument "filter" to "subpath" --- cellxgene_gateway/filecrawl.py | 8 ++++---- cellxgene_gateway/items/file/fileitem_source.py | 4 ++-- cellxgene_gateway/items/item_source.py | 2 +- cellxgene_gateway/items/s3/s3item_source.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cellxgene_gateway/filecrawl.py b/cellxgene_gateway/filecrawl.py index f812091..2ae9487 100644 --- a/cellxgene_gateway/filecrawl.py +++ b/cellxgene_gateway/filecrawl.py @@ -60,8 +60,8 @@ def render_item_tree(item_tree, item_source): return html -def render_item_source(item_source, filter=None): - item_tree = item_source.list_items(filter) - filterpart = "" if filter is None else ":" + filter - heading = f"
{item_source.name}{filterpart}
" +def render_item_source(item_source, path=None): + item_tree = item_source.list_items(path) + path_part = "" if path is None else ":" + path + heading = f"
{item_source.name}{path_part}
" return heading + render_item_tree(item_tree, item_source) diff --git a/cellxgene_gateway/items/file/fileitem_source.py b/cellxgene_gateway/items/file/fileitem_source.py index 4ade4b9..22f6ce4 100644 --- a/cellxgene_gateway/items/file/fileitem_source.py +++ b/cellxgene_gateway/items/file/fileitem_source.py @@ -50,8 +50,8 @@ class FileItemSource(ItemSource): def get_annotations_subpath(self, item) -> str: return self.convert_h5ad_path_to_annotation(item.descriptor) - def list_items(self, filter: str = None) -> ItemTree: - item_tree = self.scan_directory("" if filter is None else filter) + def list_items(self, subpath: str = None) -> ItemTree: + item_tree = self.scan_directory("" if subpath is None else subpath) """def get_items(dir): if dir.branches: diff --git a/cellxgene_gateway/items/item_source.py b/cellxgene_gateway/items/item_source.py index eb778ff..fbc26cc 100644 --- a/cellxgene_gateway/items/item_source.py +++ b/cellxgene_gateway/items/item_source.py @@ -21,7 +21,7 @@ class LookupResult: class ItemSource(ABC): @abstractmethod - def list_items(self, filter: str = None) -> List[Item]: + def list_items(self, subpath: str = None) -> List[Item]: raise Exception('"list_items" unimplemented') @abstractmethod diff --git a/cellxgene_gateway/items/s3/s3item_source.py b/cellxgene_gateway/items/s3/s3item_source.py index 130df19..a98fc16 100644 --- a/cellxgene_gateway/items/s3/s3item_source.py +++ b/cellxgene_gateway/items/s3/s3item_source.py @@ -72,8 +72,8 @@ class S3ItemSource(ItemSource): def get_annotations_subpath(self, item) -> str: return self.convert_h5ad_key_to_annotation(item.descriptor) - def list_items(self, filter: str = None) -> ItemTree: - item_tree = self.scan_directory("" if filter is None else filter) + def list_items(self, subpath: str = None) -> ItemTree: + item_tree = self.scan_directory("" if subpath is None else subpath) return item_tree @property From 6ef82b36f191d100f25acaa5dd3a86e3f1a1f8a0 Mon Sep 17 00:00:00 2001 From: Andreas Eisenbarth Date: Wed, 12 Oct 2022 16:36:46 +0200 Subject: [PATCH 2/6] For running individual tests, make sure flask_util.view_url is callable --- tests/test_filecrawl.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py index 349d1cc..c5932e0 100644 --- a/tests/test_filecrawl.py +++ b/tests/test_filecrawl.py @@ -48,6 +48,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" From 88b9b815c00b7b80920633b239ee86c855d3107d Mon Sep 17 00:00:00 2001 From: Andreas Eisenbarth Date: Wed, 12 Oct 2022 16:37:33 +0200 Subject: [PATCH 3/6] Adjust test case for dirs with h5ad --- tests/test_filecrawl.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py index c5932e0..f5c88b7 100644 --- a/tests/test_filecrawl.py +++ b/tests/test_filecrawl.py @@ -57,9 +57,16 @@ class TestRenderItemTree(unittest.TestCase): @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_tree = ItemTree("foo/bar/baz", [], []) + 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], []) rendered = render_item_tree(item_tree, item_source) self.assertEqual( rendered, - "
  • baz
      • ", + "
    • baz
      • ", ) From 0e92b7334720f285528ea5719f72daccefbc2fda Mon Sep 17 00:00:00 2001 From: Andreas Eisenbarth Date: Wed, 12 Oct 2022 16:38:14 +0200 Subject: [PATCH 4/6] Add test case for dirs without h5ad --- tests/test_filecrawl.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py index f5c88b7..c626a0c 100644 --- a/tests/test_filecrawl.py +++ b/tests/test_filecrawl.py @@ -1,4 +1,5 @@ import unittest +from collections import defaultdict from unittest.mock import MagicMock, patch from cellxgene_gateway.filecrawl import ( @@ -70,3 +71,18 @@ class TestRenderItemTree(unittest.TestCase): " | annotations: new" "", ) + + @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, + "
    • foo
        • ", + ) From 6607b15085e05a76c17a529968619089363712c2 Mon Sep 17 00:00:00 2001 From: Andreas Eisenbarth Date: Wed, 12 Oct 2022 16:57:01 +0200 Subject: [PATCH 5/6] Exclude directories having no h5ad files --- cellxgene_gateway/items/file/fileitem_source.py | 5 ++++- cellxgene_gateway/items/s3/s3item_source.py | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cellxgene_gateway/items/file/fileitem_source.py b/cellxgene_gateway/items/file/fileitem_source.py index 22f6ce4..1028ef4 100644 --- a/cellxgene_gateway/items/file/fileitem_source.py +++ b/cellxgene_gateway/items/file/fileitem_source.py @@ -63,7 +63,7 @@ class FileItemSource(ItemSource): return item_tree - def scan_directory(self, subpath="") -> dict: + def scan_directory(self, subpath: str = "") -> ItemTree: base_path = os.path.join(self.base_path, subpath) if not os.path.exists(base_path): @@ -100,6 +100,9 @@ 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) diff --git a/cellxgene_gateway/items/s3/s3item_source.py b/cellxgene_gateway/items/s3/s3item_source.py index a98fc16..0a18ad5 100644 --- a/cellxgene_gateway/items/s3/s3item_source.py +++ b/cellxgene_gateway/items/s3/s3item_source.py @@ -116,6 +116,7 @@ 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) From 624d1f856770fcb794d26857c4bf5d0bcd287f76 Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Sun, 9 Jul 2023 08:11:51 -0400 Subject: [PATCH 6/6] #78 Revert "Rename argument "filter" to "subpath"" This reverts commit fdd6cca29704a13ff8c9cd17e98971c999e0a91d. --- cellxgene_gateway/cache_entry.py | 1 - cellxgene_gateway/filecrawl.py | 8 ++++---- cellxgene_gateway/gateway.py | 2 -- cellxgene_gateway/items/file/fileitem_source.py | 8 +++++--- cellxgene_gateway/items/item_source.py | 2 +- cellxgene_gateway/items/s3/s3item_source.py | 8 +++++--- tests/test_filecrawl.py | 12 ++++++++++-- 7 files changed, 25 insertions(+), 16 deletions(-) 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/filecrawl.py b/cellxgene_gateway/filecrawl.py index 2ae9487..f812091 100644 --- a/cellxgene_gateway/filecrawl.py +++ b/cellxgene_gateway/filecrawl.py @@ -60,8 +60,8 @@ def render_item_tree(item_tree, item_source): return html -def render_item_source(item_source, path=None): - item_tree = item_source.list_items(path) - path_part = "" if path is None else ":" + path - heading = f"
        {item_source.name}{path_part}
        " +def render_item_source(item_source, filter=None): + item_tree = item_source.list_items(filter) + filterpart = "" if filter is None else ":" + filter + heading = f"
        {item_source.name}{filterpart}
        " return heading + render_item_tree(item_tree, item_source) 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 1028ef4..9addc5d 100644 --- a/cellxgene_gateway/items/file/fileitem_source.py +++ b/cellxgene_gateway/items/file/fileitem_source.py @@ -50,8 +50,8 @@ class FileItemSource(ItemSource): def get_annotations_subpath(self, item) -> str: return self.convert_h5ad_path_to_annotation(item.descriptor) - def list_items(self, subpath: str = None) -> ItemTree: - item_tree = self.scan_directory("" if subpath is None else subpath) + def list_items(self, filter: str = None) -> ItemTree: + item_tree = self.scan_directory("" if filter is None else filter) """def get_items(dir): if dir.branches: @@ -102,7 +102,9 @@ class FileItemSource(ItemSource): ] # 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] + branches = [ + branch for branch in branches if branch.items or branch.branches + ] return ItemTree(subpath, items, branches) diff --git a/cellxgene_gateway/items/item_source.py b/cellxgene_gateway/items/item_source.py index fbc26cc..eb778ff 100644 --- a/cellxgene_gateway/items/item_source.py +++ b/cellxgene_gateway/items/item_source.py @@ -21,7 +21,7 @@ class LookupResult: class ItemSource(ABC): @abstractmethod - def list_items(self, subpath: str = None) -> List[Item]: + def list_items(self, filter: str = None) -> List[Item]: raise Exception('"list_items" unimplemented') @abstractmethod diff --git a/cellxgene_gateway/items/s3/s3item_source.py b/cellxgene_gateway/items/s3/s3item_source.py index 0a18ad5..e186e62 100644 --- a/cellxgene_gateway/items/s3/s3item_source.py +++ b/cellxgene_gateway/items/s3/s3item_source.py @@ -72,8 +72,8 @@ class S3ItemSource(ItemSource): def get_annotations_subpath(self, item) -> str: return self.convert_h5ad_key_to_annotation(item.descriptor) - def list_items(self, subpath: str = None) -> ItemTree: - item_tree = self.scan_directory("" if subpath is None else subpath) + def list_items(self, filter: str = None) -> ItemTree: + item_tree = self.scan_directory("" if filter is None else filter) return item_tree @property @@ -116,7 +116,9 @@ 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] + branches = [ + branch for branch in branches if branch.items or branch.branches + ] return ItemTree(directory_key, items, branches) diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py index c626a0c..c64df8a 100644 --- a/tests/test_filecrawl.py +++ b/tests/test_filecrawl.py @@ -51,6 +51,7 @@ 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() @@ -72,9 +73,16 @@ class TestRenderItemTree(unittest.TestCase): "", ) - @patch("os.listdir", side_effect=lambda parent: defaultdict(list, {"tmp": ["foo"], "tmp/foo": ["bar"]})[parent]) + @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): + def test_GIVEN_dirs_without_h5ad_THEN_excludes_dirs_in_output( + self, listdir, exists + ): # Directories: # - tmp # - foo