From 24ea24c406119da74c6daac9ed3001ad284aeb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20L=C3=A9ger?= Date: Thu, 6 Dec 2018 16:26:45 -0500 Subject: [PATCH 1/5] Skip non-numeric value in numeric field --- src/formpack/schema/fields.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index 0454f93e..bae8846e 100644 --- a/src/formpack/schema/fields.py +++ b/src/formpack/schema/fields.py @@ -6,6 +6,7 @@ from operator import itemgetter from functools import partial +import logging try: xrange = xrange @@ -511,10 +512,15 @@ def get_disaggregated_stats(self, metrics, top_splitters, return stats def parse_values(self, raw_values): - if self.data_type == "integer": - yield int(raw_values) - else: - yield float(raw_values) + try: + if self.data_type == "integer": + yield int(raw_values) + else: + yield float(raw_values) + except ValueError as e: + # TODO Remove try/except when https://github.com/kobotoolbox/formpack/issues/151 is fixed + logging.warning(str(e), exc_info=True) + yield None class CopyField(FormField): From 121e13bfcecf5633234e4c14d8a92b905301f532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20L=C3=A9ger?= Date: Thu, 6 Dec 2018 16:27:20 -0500 Subject: [PATCH 2/5] PEP-8: Too many blank lines --- src/formpack/pack.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/formpack/pack.py b/src/formpack/pack.py index 3f5cfa3d..8790e443 100644 --- a/src/formpack/pack.py +++ b/src/formpack/pack.py @@ -229,7 +229,6 @@ def get_fields_for_versions(self, versions=-1, data_types=None): if isinstance(data_types, str_types): data_types = [data_types] - # tmp2 is a 2 dimensions list of `field`. # First dimension is the position of fields where they should be in the latest version # Second dimension is their position in the stack at the same position. From bf3debe31cef00b81e952a75e08b90d12801f773 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20L=C3=A9ger?= Date: Thu, 6 Dec 2018 16:47:01 -0500 Subject: [PATCH 3/5] Removed useless yield --- src/formpack/schema/fields.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index bae8846e..637b3f4f 100644 --- a/src/formpack/schema/fields.py +++ b/src/formpack/schema/fields.py @@ -520,7 +520,6 @@ def parse_values(self, raw_values): except ValueError as e: # TODO Remove try/except when https://github.com/kobotoolbox/formpack/issues/151 is fixed logging.warning(str(e), exc_info=True) - yield None class CopyField(FormField): From f5e1d0bd414b803ae8818a79d7569a3214956fe1 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 4 Feb 2019 09:27:02 -0500 Subject: [PATCH 4/5] Add failing unit test for #185 --- tests/test_autoreport.py | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/test_autoreport.py b/tests/test_autoreport.py index b3381ffa..a4100909 100644 --- a/tests/test_autoreport.py +++ b/tests/test_autoreport.py @@ -381,3 +381,51 @@ def test_disaggregate_extended_fields(self): frequency_responses = [x[0] for x in value_list.get("frequency")] assert percentage_responses == frequency_responses assert percentage_responses[-1] == "..." + + def test_stats_with_non_numeric_value_for_numeric_field(self): + ''' + A string response to an integer question, for example, should not cause + a crash; it should be treated as if no response was provided + ''' + + title = 'Just one number' + schemas = [{ + 'content': { + 'survey': [ + { + 'type': 'integer', + 'name': 'the_number', + 'label': 'Enter the number!' + } + ] + } + }] + submissions = [ + {'the_number': 10}, + {'the_number': 20}, + {'the_number': 30}, + {'the_number': 'oops!'}, + ] + fp = FormPack(schemas, title) + + report = fp.autoreport() + stats = report.get_stats(submissions) + + assert stats.submissions_count == len(submissions) + + stats = [(unicode(repr(f)), n, d) for f, n, d in stats] + expected = [( + "", 'the_number', + { + 'mean': 20.0, + 'median': 20, + 'mode': u'*', + 'not_provided': 1, + 'provided': 3, + 'show_graph': False, + 'stdev': 10.0, + 'total_count': 4 + } + )] + for (i, stat) in enumerate(stats): + assert stat == expected[i] From 02a257b190e1732ace4db79423ed9cc822543d55 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 4 Feb 2019 09:27:32 -0500 Subject: [PATCH 5/5] Move ValueError handling into AutoReport so... that responses of the wrong type can be treated as "not provided" --- src/formpack/reporting/autoreport.py | 16 +++++++++++++--- src/formpack/schema/fields.py | 13 ++++--------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/formpack/reporting/autoreport.py b/src/formpack/reporting/autoreport.py index b2e9a9b9..2d5b68de 100644 --- a/src/formpack/reporting/autoreport.py +++ b/src/formpack/reporting/autoreport.py @@ -2,6 +2,7 @@ from __future__ import (division, print_function, unicode_literals) +import logging from collections import Counter, defaultdict from ..constants import UNSPECIFIED_TRANSLATION @@ -70,9 +71,18 @@ def _calculate_stats(self, submissions, fields, versions, lang): counter = metrics[field.name] raw_value = entry.get(field.path) if raw_value is not None: - values = list(field.parse_values(raw_value)) - counter.update(values) - counter['__submissions__'] += 1 + try: + values = list(field.parse_values(raw_value)) + except ValueError as e: + # TODO: Remove try/except when + # https://github.com/kobotoolbox/formpack/issues/151 + # is fixed? + logging.warning(str(e), exc_info=True) + # Treat the bad value as a blank response + counter[None] += 1 + else: + counter.update(values) + counter['__submissions__'] += 1 else: counter[None] += 1 diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index 637b3f4f..0454f93e 100644 --- a/src/formpack/schema/fields.py +++ b/src/formpack/schema/fields.py @@ -6,7 +6,6 @@ from operator import itemgetter from functools import partial -import logging try: xrange = xrange @@ -512,14 +511,10 @@ def get_disaggregated_stats(self, metrics, top_splitters, return stats def parse_values(self, raw_values): - try: - if self.data_type == "integer": - yield int(raw_values) - else: - yield float(raw_values) - except ValueError as e: - # TODO Remove try/except when https://github.com/kobotoolbox/formpack/issues/151 is fixed - logging.warning(str(e), exc_info=True) + if self.data_type == "integer": + yield int(raw_values) + else: + yield float(raw_values) class CopyField(FormField):