From f4af15c94411e088e85a380f87f02b30a1deaa32 Mon Sep 17 00:00:00 2001 From: Tim Gion Date: Wed, 19 Apr 2017 16:06:51 +0000 Subject: [PATCH 1/4] Setup tests with proper linking of channels. Annotation channels should have a source channel. --- django/bosscore/test/setup_db.py | 37 ++++++++++++++----- django/bosscore/test/test_auth.py | 2 +- .../bossspatialdb/test/test_djangoresource.py | 2 +- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/django/bosscore/test/setup_db.py b/django/bosscore/test/setup_db.py index 7ddf605e..ca809f1e 100644 --- a/django/bosscore/test/setup_db.py +++ b/django/bosscore/test/setup_db.py @@ -19,6 +19,7 @@ from guardian.shortcuts import assign_perm from ..models import Collection, Experiment, CoordinateFrame, Channel, BossLookup, BossRole, BossGroup +from ..views.views_resource import ChannelDetail from spdb.spatialdb.test.setup import AWSSetupLayer @@ -97,8 +98,8 @@ def insert_test_data(self): self.add_experiment('col1', 'exp22', 'cf1', 10, 500, 1) self.add_channel('col1', 'exp1', 'channel1', 0, 0, 'uint8', 'image') self.add_channel('col1', 'exp1', 'channel2', 0, 0, 'uint8', 'image') - self.add_channel('col1', 'exp1', 'channel3', 0, 0, 'uint64', 'annotation') - self.add_channel('col1', 'exp1', 'layer1', 0, 0, 'uint64', 'annotation') + self.add_channel('col1', 'exp1', 'channel3', 0, 0, 'uint64', 'annotation', ['channel1']) + self.add_channel('col1', 'exp1', 'layer1', 0, 0, 'uint64', 'annotation', ['channel1']) def insert_spatialdb_test_data(self): @@ -107,9 +108,9 @@ def insert_spatialdb_test_data(self): self.add_experiment('col1', 'exp1', 'cf1', 10, 500, 1) self.add_channel('col1', 'exp1', 'channel1', 0, 0, 'uint8', 'image') self.add_channel('col1', 'exp1', 'channel2', 0, 0, 'uint16', 'image') - self.add_channel('col1', 'exp1', 'layer1', 0, 0, 'uint64', 'annotation') + self.add_channel('col1', 'exp1', 'layer1', 0, 0, 'uint64', 'annotation', ['channel1']) # bbchan1 is a channel for bounding box tests. - self.add_channel('col1', 'exp1', 'bbchan1', 0, 0, 'uint64', 'annotation') + self.add_channel('col1', 'exp1', 'bbchan1', 0, 0, 'uint64', 'annotation', ['channel1']) def insert_ingest_test_data(self): @@ -252,17 +253,19 @@ def add_experiment(self, collection_name, experiment_name, coordinate_name, num_ return exp def add_channel(self, collection_name, experiment_name, channel_name, - default_time_sample, base_resolution, datatype, channel_type=None): + default_time_sample, base_resolution, datatype, + channel_type=None, source_channels=[]): """ Args: - collection_name: Name of the collection - experiment_name: Name of the experiment - channel_name: Name of the channel + collection_name (string): Name of the collection + experiment_name (string): Name of the experiment + channel_name (string): Name of the channel default_time_sample: Default time sample base_resolution: Base resolution of the channel - datatype: Data type - channel_type: Channel Type (image or annotation) + datatype (string): Data type + channel_type (string): Channel Type (image or annotation) + source_channels (list[string]): Source channel(s) for an annotation channel Returns: Channel @@ -270,12 +273,26 @@ def add_channel(self, collection_name, experiment_name, channel_name, """ if channel_type is None: channel_type = 'image' + elif channel_type == 'annotation' and len(source_channels) == 0: + raise Exception('Annotation channel must have source channel.') + + # Not setting up any related channels. + related_channels = [] + col = Collection.objects.get(name=collection_name) exp = Experiment.objects.get(name=experiment_name, collection=col) channel = Channel.objects.create(name=channel_name, experiment=exp, default_time_sample=default_time_sample, base_resolution=base_resolution, type=channel_type, datatype=datatype, creator=self.user) + src_chan_objs, rel_chan_objs = ChannelDetail.validate_source_related_channels( + exp, source_channels, related_channels) + + # Add source channels. + channel = ChannelDetail.add_source_related_channels( + channel, exp, src_chan_objs, rel_chan_objs) + + # Set lookup key. base_lkup_key = str(col.pk) + '&' + str(exp.pk) + '&' + str(channel.pk) base_bs_key = col.name + '&' + exp.name + '&' + channel.name BossLookup.objects.create(lookup_key=base_lkup_key, boss_key=base_bs_key, diff --git a/django/bosscore/test/test_auth.py b/django/bosscore/test/test_auth.py index d3c014ef..ed481336 100644 --- a/django/bosscore/test/test_auth.py +++ b/django/bosscore/test/test_auth.py @@ -430,7 +430,7 @@ def setUp(self): dbsetup.add_experiment('unittestcol', 'unittestexp', 'unittestcf', 10, 10, 1) dbsetup.add_channel('unittestcol', 'unittestexp', 'unittestchannel', 0, 0, 'uint8') - dbsetup.add_channel('unittestcol', 'unittestexp', 'unittestlayer', 0, 0, 'uint16', 'annotation') + dbsetup.add_channel('unittestcol', 'unittestexp', 'unittestlayer', 0, 0, 'uint16', 'annotation', ['unittestchannel']) def test_get_channel_no_permission(self): """ diff --git a/django/bossspatialdb/test/test_djangoresource.py b/django/bossspatialdb/test/test_djangoresource.py index 329ae3f5..cc4d2578 100644 --- a/django/bossspatialdb/test/test_djangoresource.py +++ b/django/bossspatialdb/test/test_djangoresource.py @@ -213,7 +213,7 @@ def test_django_resource_channel_annotation(self): assert channel.base_resolution == self.request_annotation.channel.base_resolution assert channel.default_time_sample == self.request_annotation.channel.default_time_sample assert channel.related == [] - assert channel.sources == [] + assert channel.sources == ['channel1'] def test_django_resource_channel_image_with_links(self): """Test basic get channel interface From 1f922b8be3f171c16c195a5d823789d44ca60124 Mon Sep 17 00:00:00 2001 From: Tim Gion Date: Wed, 19 Apr 2017 16:07:57 +0000 Subject: [PATCH 2/4] Unit test additional delete channel case. If all of a channel's derived channels are marked for deletion, then should be able to delete the channel. --- django/bosscore/test/test_resource_views.py | 32 +++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/django/bosscore/test/test_resource_views.py b/django/bosscore/test/test_resource_views.py index 7c9398c7..f0581dd0 100644 --- a/django/bosscore/test/test_resource_views.py +++ b/django/bosscore/test/test_resource_views.py @@ -1073,7 +1073,7 @@ def test_put_channel_name(self): def test_delete_channel(self): """ - Delete a experiment + Delete a channel """ # Post a new channel @@ -1107,11 +1107,39 @@ def test_delete_channel_invalid(self): response = self.client.delete(url) self.assertEqual(response.status_code, 400) - # Get an existing experiment + # Ensure channel still exists url = '/' + version + '/collection/col1/experiment/exp1/channel/channel1/' response = self.client.get(url) self.assertEqual(response.status_code, 200) + def test_delete_channel_ignore_derived_channels_marked_for_deletion(self): + """ + Delete a channel (allow when all derived channels are marked for deletion) + + """ + + # Post a new channel + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel33/' + data = {'description': 'This is a new channel', 'type': 'annotation', 'datatype': 'uint64', + 'sources': ['channel1'], 'related': ['channel2']} + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 201) + + # Delete the new channel + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel33/' + response = self.client.delete(url, data=data) + self.assertEqual(response.status_code, 204) + + # Delete the source channel + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel1' + response = self.client.delete(url) + self.assertEqual(response.status_code, 204) + + # Delete the related channel + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel2' + response = self.client.delete(url) + self.assertEqual(response.status_code, 204) + def test_delete_channel_doesnotexist(self): """ Delete a channel (invalid - The channel does not exist ) From 3ca07d99b1e7777373fc499110733c310a014799 Mon Sep 17 00:00:00 2001 From: Tim Gion Date: Wed, 19 Apr 2017 17:09:33 +0000 Subject: [PATCH 3/4] Fix for deleting channels. Allow deleting a channel when all of its derived channels are marked for deletion. Derived channels are channels that list the channel as either a source or related channel. --- django/bosscore/models.py | 10 +++++++++- django/bosscore/test/test_resource_views.py | 18 ++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/django/bosscore/models.py b/django/bosscore/models.py index a31cb14f..c305d0d3 100644 --- a/django/bosscore/models.py +++ b/django/bosscore/models.py @@ -248,7 +248,15 @@ def remove_source(self, source): return def get_derived(self): - derived = Source.objects.filter(source_channel=self) + """ + Get channels that list this channel as either a source or related channel. + + Do not return any channels that are marked for deletion. + + Returns: + (list-like object of Source objects) + """ + derived = Source.objects.filter(source_channel=self).exclude(derived_channel__to_be_deleted__isnull=False) return derived def __str__(self): diff --git a/django/bosscore/test/test_resource_views.py b/django/bosscore/test/test_resource_views.py index f0581dd0..81e2653f 100644 --- a/django/bosscore/test/test_resource_views.py +++ b/django/bosscore/test/test_resource_views.py @@ -1118,10 +1118,20 @@ def test_delete_channel_ignore_derived_channels_marked_for_deletion(self): """ - # Post a new channel + # Post new channels + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel11/' + data = {'description': 'This is a new source channel', 'type': 'image', 'datatype': 'uint8'} + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 201) + + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel22/' + data = {'description': 'This is a new related channel', 'type': 'image', 'datatype': 'uint8'} + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 201) + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel33/' data = {'description': 'This is a new channel', 'type': 'annotation', 'datatype': 'uint64', - 'sources': ['channel1'], 'related': ['channel2']} + 'sources': ['channel11'], 'related': ['channel22']} response = self.client.post(url, data=data) self.assertEqual(response.status_code, 201) @@ -1131,12 +1141,12 @@ def test_delete_channel_ignore_derived_channels_marked_for_deletion(self): self.assertEqual(response.status_code, 204) # Delete the source channel - url = '/' + version + '/collection/col1/experiment/exp1/channel/channel1' + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel11' response = self.client.delete(url) self.assertEqual(response.status_code, 204) # Delete the related channel - url = '/' + version + '/collection/col1/experiment/exp1/channel/channel2' + url = '/' + version + '/collection/col1/experiment/exp1/channel/channel22' response = self.client.delete(url) self.assertEqual(response.status_code, 204) From 89c7f4b3a18fcb3023221e83bb9e9863df417d9a Mon Sep 17 00:00:00 2001 From: Tim Gion Date: Thu, 20 Apr 2017 09:40:24 -0400 Subject: [PATCH 4/4] Update docstrings as suggested. Use str instead of string. Channel.get_derived()'s exact return type is a QuerySet. --- django/bosscore/models.py | 2 +- django/bosscore/test/setup_db.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/django/bosscore/models.py b/django/bosscore/models.py index c305d0d3..77fd428f 100644 --- a/django/bosscore/models.py +++ b/django/bosscore/models.py @@ -254,7 +254,7 @@ def get_derived(self): Do not return any channels that are marked for deletion. Returns: - (list-like object of Source objects) + (QuerySet) """ derived = Source.objects.filter(source_channel=self).exclude(derived_channel__to_be_deleted__isnull=False) return derived diff --git a/django/bosscore/test/setup_db.py b/django/bosscore/test/setup_db.py index ca809f1e..5771e793 100644 --- a/django/bosscore/test/setup_db.py +++ b/django/bosscore/test/setup_db.py @@ -258,14 +258,14 @@ def add_channel(self, collection_name, experiment_name, channel_name, """ Args: - collection_name (string): Name of the collection - experiment_name (string): Name of the experiment - channel_name (string): Name of the channel + collection_name (str): Name of the collection + experiment_name (str): Name of the experiment + channel_name (str): Name of the channel default_time_sample: Default time sample base_resolution: Base resolution of the channel - datatype (string): Data type - channel_type (string): Channel Type (image or annotation) - source_channels (list[string]): Source channel(s) for an annotation channel + datatype (str): Data type + channel_type (str): Channel Type (image or annotation) + source_channels (list[str]): Source channel(s) for an annotation channel Returns: Channel