Skip to content

Commit

Permalink
Handle JSON numbers as Decimal not float
Browse files Browse the repository at this point in the history
This is what flatten-tool does, and it avoids rounding errors in our
sums (e.g. total amount awarded).
  • Loading branch information
Bjwebb committed Jul 21, 2017
1 parent 2cba86c commit fb11bed
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 8 deletions.
25 changes: 25 additions & 0 deletions cove/lib/tools.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import datetime
import strict_rfc3339
from functools import wraps # use this to preserve function signatures and docstrings
from decimal import Decimal

from . exceptions import UnrecognisedFileType

Expand Down Expand Up @@ -67,3 +68,27 @@ def get_file_type(file):
return 'json'
else:
raise UnrecognisedFileType


# From http://bugs.python.org/issue16535
class NumberStr(float):
def __init__(self, o):
# We don't call the parent here, since we're deliberately altering it's functionality
# pylint: disable=W0231
self.o = o

def __repr__(self):
return str(self.o)

# This is needed for this trick to work in python 3.4
def __float__(self):
return self


def decimal_default(o):
if isinstance(o, Decimal):
if int(o) == o:
return int(o)
else:
return NumberStr(o)
raise TypeError(repr(o) + " is not JSON serializable")
4 changes: 2 additions & 2 deletions cove/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import cove.lib.common as common
from cove.input.models import SuppliedData
from cove.lib.tools import get_file_type
from cove.lib.tools import get_file_type, decimal_default

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -94,7 +94,7 @@ def common_checks_context(data, json_data, schema_obj, schema_name, context, ext
cell_source_map, heading_source_map,
extra_checkers=extra_checkers)
with open(validation_errors_path, 'w+') as validiation_error_fp:
validiation_error_fp.write(json.dumps(validation_errors))
validiation_error_fp.write(json.dumps(validation_errors, default=decimal_default))

extensions = None
if getattr(schema_obj, 'extensions', None):
Expand Down
8 changes: 8 additions & 0 deletions cove_360/fixtures/decimal_amounts.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Currency,Amount Awarded
GBP,1000.1
GBP,1000.1
GBP,1000.1
GBP,1000.1
GBP,1000.1
GBP,1000.1
GBP,1000.1
32 changes: 32 additions & 0 deletions cove_360/fixtures/decimal_amounts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"grants": [
{
"currency": "GBP",
"amountAwarded": 1000.1
},
{
"currency": "GBP",
"amountAwarded": 1000.1
},
{
"currency": "GBP",
"amountAwarded": 1000.1
},
{
"currency": "GBP",
"amountAwarded": 1000.1
},
{
"currency": "GBP",
"amountAwarded": 1000.1
},
{
"currency": "GBP",
"amountAwarded": 1000.1
},
{
"currency": "GBP",
"amountAwarded": 1000.1
}
]
}
3 changes: 2 additions & 1 deletion cove_360/lib/threesixtygiving.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import re
import os
from collections import defaultdict, OrderedDict
from decimal import Decimal

import requests

Expand Down Expand Up @@ -55,7 +56,7 @@ def get_grants_aggregates(json_data):

currencies[currency]["count"] += 1
amount_awarded = grant.get('amountAwarded')
if amount_awarded and isinstance(amount_awarded, (int, float)):
if amount_awarded and isinstance(amount_awarded, (int, Decimal, float)):
currencies[currency]["total_amount"] += amount_awarded
currencies[currency]['max_amount'] = max(amount_awarded, currencies[currency]['max_amount'])
if not currencies[currency]["min_amount"]:
Expand Down
4 changes: 3 additions & 1 deletion cove_360/tests_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ def server_url(request, live_server):
'Value is not a number',
'Value is not a string',
], True),
('decimal_amounts.csv', 'The grants were awarded in GBP with a total value of £7,000.7 and individual awards ranging from £1,000.1 (lowest) to £1,000.1 (highest).', True),
('decimal_amounts.json', 'The grants were awarded in GBP with a total value of £7,000.7 and individual awards ranging from £1,000.1 (lowest) to £1,000.1 (highest).', True),
])
def test_explore_360_url_input(server_url, browser, httpserver, source_filename, expected_text, conversion_successful):
Expand Down Expand Up @@ -166,7 +168,7 @@ def check_url_input_result_page(server_url, browser, httpserver, source_filename
assert "unflattened.json" in browser.find_element_by_link_text("JSON (Converted from Original)").get_attribute("href")

assert source_filename in original_file
assert '0 bytes' not in body_text
assert ' 0 bytes' not in body_text
# Test for Load New File button
assert 'Load New File' in body_text

Expand Down
5 changes: 3 additions & 2 deletions cove_360/views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import logging
from decimal import Decimal

from django.shortcuts import render
from django.utils.translation import ugettext_lazy as _
Expand Down Expand Up @@ -45,7 +46,7 @@ def explore_360(request, pk, template='cove_360/explore.html'):
# open the data first so we can inspect for record package
with open(db_data.original_file.file.name, encoding='utf-8') as fp:
try:
json_data = json.load(fp)
json_data = json.load(fp, parse_float=Decimal)
except ValueError as err:
raise CoveInputDataError(context={
'sub_title': _("Sorry we can't process that data"),
Expand All @@ -69,7 +70,7 @@ def explore_360(request, pk, template='cove_360/explore.html'):
else:
context.update(convert_spreadsheet(request, db_data, file_type, schema_360.release_schema_url))
with open(context['converted_path'], encoding='utf-8') as fp:
json_data = json.load(fp)
json_data = json.load(fp, parse_float=Decimal)

context = common_checks_360(context, db_data, json_data, schema_360)

Expand Down
5 changes: 3 additions & 2 deletions cove_ocds/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import os
import re
from decimal import Decimal

from django.shortcuts import render
from django.utils.translation import ugettext_lazy as _
Expand Down Expand Up @@ -90,7 +91,7 @@ def explore_ocds(request, pk):
# open the data first so we can inspect for record package
with open(db_data.original_file.file.name, encoding='utf-8') as fp:
try:
json_data = json.load(fp)
json_data = json.load(fp, parse_float=Decimal)
except ValueError as err:
raise CoveInputDataError(context={
'sub_title': _("Sorry we can't process that data"),
Expand Down Expand Up @@ -173,7 +174,7 @@ def explore_ocds(request, pk):
context.update(convert_spreadsheet(request, db_data, file_type, schema_url=url, pkg_schema_url=pkg_url, replace=replace))

with open(context['converted_path'], encoding='utf-8') as fp:
json_data = json.load(fp)
json_data = json.load(fp, parse_float=Decimal)

if replace:
if os.path.exists(validation_errors_path):
Expand Down

0 comments on commit fb11bed

Please sign in to comment.