From a64956f3f05fcfa505ac2a08efb067e6cd85e636 Mon Sep 17 00:00:00 2001 From: zxenia Date: Fri, 29 Apr 2022 11:40:31 -0400 Subject: [PATCH 1/4] add type exception to public overview --- chord_metadata_service/restapi/api_views.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/chord_metadata_service/restapi/api_views.py b/chord_metadata_service/restapi/api_views.py index 9a17b0c5e..fc603190d 100644 --- a/chord_metadata_service/restapi/api_views.py +++ b/chord_metadata_service/restapi/api_views.py @@ -1,4 +1,6 @@ import math +import logging + from collections import Counter from django.conf import settings from django.views.decorators.cache import cache_page @@ -17,6 +19,10 @@ from chord_metadata_service.mcode.api_views import MCODEPACKET_PREFETCH, MCODEPACKET_SELECT +logger = logging.getLogger("restapi_api_views") +logger.setLevel(logging.INFO) + + @api_view() @permission_classes([AllowAny]) def service_info(_request): @@ -339,7 +345,10 @@ def public_overview(_request): # add new Counter() if key not in extra_properties: extra_properties[key] = Counter() - extra_properties[key].update((individual.extra_properties[key],)) + try: + extra_properties[key].update((individual.extra_properties[key],)) + except TypeError: + logger.error(f"The extra_properties {key} value is not of type string or number.") individuals_extra_properties[key] = dict(extra_properties[key]) # Experiments for experiment in experiments: From 7991db445d935e8b1f7efab707ac711af7668824 Mon Sep 17 00:00:00 2001 From: zxenia Date: Fri, 29 Apr 2022 16:04:01 -0400 Subject: [PATCH 2/4] add check for collected values for public_overview --- chord_metadata_service/restapi/api_views.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/chord_metadata_service/restapi/api_views.py b/chord_metadata_service/restapi/api_views.py index fc603190d..2a0d4d3db 100644 --- a/chord_metadata_service/restapi/api_views.py +++ b/chord_metadata_service/restapi/api_views.py @@ -349,6 +349,8 @@ def public_overview(_request): extra_properties[key].update((individual.extra_properties[key],)) except TypeError: logger.error(f"The extra_properties {key} value is not of type string or number.") + pass + individuals_extra_properties[key] = dict(extra_properties[key]) # Experiments for experiment in experiments: @@ -376,13 +378,14 @@ def public_overview(_request): if "bin_size" in settings.CONFIG_FIELDS["extra_properties"][key] else None # retrieve the values from extra_properties counter values = individuals_extra_properties[key] - kwargs = dict(values=values, bin_size=field_bin_size) - # sort into bins and remove numeric values where count <= threshold - extra_prop_values_in_bins = sort_numeric_values_into_bins( - **{k: v for k, v in kwargs.items() if v is not None} - ) - # rewrite with sorted values - individuals_extra_properties[key] = extra_prop_values_in_bins + if values: + kwargs = dict(values=values, bin_size=field_bin_size) + # sort into bins and remove numeric values where count <= threshold + extra_prop_values_in_bins = sort_numeric_values_into_bins( + **{k: v for k, v in kwargs.items() if v is not None} + ) + # rewrite with sorted values + individuals_extra_properties[key] = extra_prop_values_in_bins # add missing value count individuals_extra_properties[key][missing] = len(individuals_set) - sum(v for v in value.values()) else: From b7536ae76d7ec6f724424f27ec1fe4bea1e26883 Mon Sep 17 00:00:00 2001 From: zxenia Date: Fri, 29 Apr 2022 16:04:58 -0400 Subject: [PATCH 3/4] add tests for not accepted data types for public overview count --- .../restapi/tests/constants.py | 37 +++++++++++++ .../restapi/tests/test_api.py | 54 ++++++++++++++++++- 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/chord_metadata_service/restapi/tests/constants.py b/chord_metadata_service/restapi/tests/constants.py index 8f8fa903d..d02a3243a 100644 --- a/chord_metadata_service/restapi/tests/constants.py +++ b/chord_metadata_service/restapi/tests/constants.py @@ -1,3 +1,6 @@ +from copy import deepcopy + + INVALID_FHIR_BUNDLE_1 = { "resourceType": "NotBundle", "entry": [ @@ -209,6 +212,40 @@ VALID_INDIVIDUALS = [VALID_INDIVIDUAL_1, VALID_INDIVIDUAL_2, VALID_INDIVIDUAL_3, VALID_INDIVIDUAL_4, VALID_INDIVIDUAL_5, VALID_INDIVIDUAL_6, VALID_INDIVIDUAL_7, VALID_INDIVIDUAL_8] + +extra_properties_with_list = { + "smoking": "Former smoker", + "covidstatus": "Positive", + "death_dc": "Alive", + "mobility": "I have slight problems in walking about", + "date_of_consent": "2021-03-03", + "lab_test_result_value": 699.86, + "baseline_creatinine": [100, 120] + } + +extra_properties_with_dict = { + "smoking": "Former smoker", + "covidstatus": "Positive", + "death_dc": "Alive", + "mobility": "I have slight problems in walking about", + "date_of_consent": "2021-03-03", + "lab_test_result_value": 699.86, + "baseline_creatinine": { + "test_key_1": 120, + "test_key_2": "test_value_2" + } + } + + +INDIVIDUALS_NOT_ACCEPTED_DATA_TYPES_LIST = [ + {**item, "extra_properties": extra_properties_with_list} for item in deepcopy(VALID_INDIVIDUALS) +] + +INDIVIDUALS_NOT_ACCEPTED_DATA_TYPES_DICT = [ + {**item, "extra_properties": extra_properties_with_dict} for item in deepcopy(VALID_INDIVIDUALS) +] + + CONFIG_FIELDS_TEST = { "sex": { "type": "string", diff --git a/chord_metadata_service/restapi/tests/test_api.py b/chord_metadata_service/restapi/tests/test_api.py index e4a96e13b..8a699c17e 100644 --- a/chord_metadata_service/restapi/tests/test_api.py +++ b/chord_metadata_service/restapi/tests/test_api.py @@ -1,4 +1,5 @@ from copy import deepcopy + from django.conf import settings from django.urls import reverse from django.test import override_settings @@ -12,9 +13,13 @@ from chord_metadata_service.experiments.tests import constants as exp_c from chord_metadata_service.mcode import models as mcode_m from chord_metadata_service.mcode.tests import constants as mcode_c -from .constants import CONFIG_FIELDS_TEST -from .constants import VALID_INDIVIDUALS +from .constants import ( + CONFIG_FIELDS_TEST, + VALID_INDIVIDUALS, + INDIVIDUALS_NOT_ACCEPTED_DATA_TYPES_LIST, + INDIVIDUALS_NOT_ACCEPTED_DATA_TYPES_DICT +) class ServiceInfoTest(APITestCase): @@ -227,3 +232,48 @@ def test_overview_response_no_config(self): response_obj = response.json() self.assertIsInstance(response_obj, dict) self.assertEqual(response_obj, settings.NO_PUBLIC_DATA_AVAILABLE) + + +class PublicOverviewNotSupportedDataTypesListTest(APITestCase): + # individuals (count 8) + def setUp(self) -> None: + # create individuals including those who have not accepted data types + for ind in INDIVIDUALS_NOT_ACCEPTED_DATA_TYPES_LIST: + ph_m.Individual.objects.create(**ind) + + @override_settings(CONFIG_FIELDS=CONFIG_FIELDS_TEST) + def test_overview_response(self): + # test overview response with passing TypeError exception + response = self.client.get('/api/public_overview') + response_obj = response.json() + print(response_obj) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsInstance(response_obj, dict) + # the field name is present, but the keys are not (except 'missing') + self.assertIn("baseline_creatinine", response_obj["extra_properties"]) + self.assertIn("missing", response_obj["extra_properties"]["baseline_creatinine"]) + self.assertEqual(8, response_obj["extra_properties"]["baseline_creatinine"]["missing"]) + # if we add support for an array values for the public_overview + # then this assertion will fail, so far there is no support for it + self.assertNotIn(100, response_obj["extra_properties"]["baseline_creatinine"]) + + +class PublicOverviewNotSupportedDataTypesDictTest(APITestCase): + # individuals (count 8) + def setUp(self) -> None: + # create individuals including those who have not accepted data types + for ind in INDIVIDUALS_NOT_ACCEPTED_DATA_TYPES_DICT: + ph_m.Individual.objects.create(**ind) + + @override_settings(CONFIG_FIELDS=CONFIG_FIELDS_TEST) + def test_overview_response(self): + # test overview response with passing TypeError exception + response = self.client.get('/api/public_overview') + response_obj = response.json() + print(response_obj) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsInstance(response_obj, dict) + # the field name is present, but the keys are not (except 'missing') + self.assertIn("baseline_creatinine", response_obj["extra_properties"]) + self.assertIn("missing", response_obj["extra_properties"]["baseline_creatinine"]) + self.assertEqual(8, response_obj["extra_properties"]["baseline_creatinine"]["missing"]) From dc666d822fd4e4ac120117fdd9f4fdc7b948c9a3 Mon Sep 17 00:00:00 2001 From: zxenia Date: Fri, 29 Apr 2022 16:16:38 -0400 Subject: [PATCH 4/4] update version in package.cfg --- chord_metadata_service/package.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chord_metadata_service/package.cfg b/chord_metadata_service/package.cfg index 66666ad57..049222625 100644 --- a/chord_metadata_service/package.cfg +++ b/chord_metadata_service/package.cfg @@ -1,4 +1,4 @@ [package] name = katsu -version = 2.9.1 +version = 2.9.2 authors = Ksenia Zaytseva, David Lougheed, Simon Chénard, Romain Grégoire