Skip to content

Commit

Permalink
Merge pull request #186 from kobotoolbox/185-autoreport-crashes-with-…
Browse files Browse the repository at this point in the history
…non-numeric-in-numeric-field

autoreport ignores non-numeric values in numeric field
  • Loading branch information
jnm authored Feb 4, 2019
2 parents dc06fd7 + ff19064 commit 09c600f
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 4 deletions.
1 change: 0 additions & 1 deletion src/formpack/pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,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.
Expand Down
16 changes: 13 additions & 3 deletions src/formpack/reporting/autoreport.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import (division, print_function, unicode_literals)

import logging
from collections import Counter, defaultdict

from ..constants import UNSPECIFIED_TRANSLATION
Expand Down Expand Up @@ -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

Expand Down
48 changes: 48 additions & 0 deletions tests/test_autoreport.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [(
"<NumField name='the_number' type='integer'>", '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]

0 comments on commit 09c600f

Please sign in to comment.