Skip to content

Commit 361a99d

Browse files
committed
Avoid floating point inaccuracies
1 parent 56e3dde commit 361a99d

3 files changed

Lines changed: 68 additions & 24 deletions

File tree

pyxform/errors.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,8 @@ class ErrorCode(Enum):
423423
name="Range type - parameter outside range",
424424
msg=(
425425
"[row : {row}] On the 'survey' sheet, the 'parameters' value is invalid. "
426-
"For the 'range' question type, the parameter '{name}' must be a within "
427-
"the range (between the 'start' and 'end' values, inclusive)."
426+
"For the 'range' question type, the parameter '{name}' must be "
427+
"between the 'start' and 'end' values, inclusive)."
428428
),
429429
)
430430
RANGE_006 = Detail(

pyxform/validators/pyxform/question_types.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
from collections.abc import Collection, Iterable
6+
from decimal import Decimal, InvalidOperation
67
from math import isinf
78
from typing import Any
89

@@ -138,14 +139,14 @@ def process_range_question_type(
138139
if key not in parameters:
139140
parameters[key] = defaults[key]
140141

141-
def process_parameter(name: str) -> float | None:
142+
def process_parameter(name: str) -> Decimal | None:
142143
value = parameters.get(name)
143144
if value is None:
144145
return value
145146
err = False
146147
try:
147-
value = float(value)
148-
except ValueError:
148+
value = Decimal(value)
149+
except InvalidOperation:
149150
err = True
150151

151152
if err or isinf(value):
@@ -202,8 +203,8 @@ def process_parameter(name: str) -> float | None:
202203
for label in tick_list:
203204
errored = False
204205
try:
205-
label = float(label.get("name"))
206-
except ValueError:
206+
label = Decimal(label.get("name"))
207+
except InvalidOperation:
207208
errored = True
208209

209210
if errored or isinf(label):
@@ -230,9 +231,9 @@ def process_parameter(name: str) -> float | None:
230231

231232
parameters["odk:tick-labelset"] = parameters.pop("tick_labelset")
232233

233-
# Default is integer, but if the floats have decimals then change the bind type.
234+
# Default is integer, but if the values have decimals then change the bind type.
234235
if any(
235-
i is not None and not i.is_integer()
236+
i is not None and not i == i.to_integral_value()
236237
for i in (start, end, step, tick_interval, placeholder)
237238
):
238239
row["bind"] = row.get("bind", {})

tests/test_range.py

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -294,40 +294,83 @@ def test_parameter_greater_than_range__ok(self):
294294
],
295295
)
296296

297-
def test_parameter_not_a_multiple_of_step__error(self):
297+
def test_tick_interval_not_a_multiple_of_step__error(self):
298298
"""Should raise an error if the relevant ticks parameter is not a multiple of 'step'."""
299299
# RI003 RP001
300300
md = """
301301
| survey |
302+
| | type | name | label | parameters |
303+
| | range | q1 | Q1 | start=-3 end=3 step=2 tick_interval={value} |
304+
"""
305+
cases = ("-3", "3", "-1", "1")
306+
for value in cases:
307+
with self.subTest(("tick_interval", value)):
308+
self.assertPyxformXform(
309+
md=md.format(name="tick_interval", value=value),
310+
errored=True,
311+
error__contains=[
312+
ErrorCode.RANGE_004.value.format(row=2, name="tick_interval")
313+
],
314+
)
315+
316+
def test_tick_interval_not_a_multiple_of_step__error(self):
317+
"""Should raise an error if the placeholder is not a multiple of 'step' starting at 'start'."""
318+
# RI003 RP001
319+
md = """
320+
| survey |
321+
| | type | name | label | parameters |
322+
| | range | q1 | Q1 | start=-3 end=3 step=2 placeholder={value} |
323+
"""
324+
cases = ("-2", "2", "0")
325+
for value in cases:
326+
with self.subTest(("placeholder", value)):
327+
self.assertPyxformXform(
328+
md=md.format(name="placeholder", value=value),
329+
errored=True,
330+
error__contains=[
331+
ErrorCode.RANGE_004.value.format(row=2, name="placeholder")
332+
],
333+
)
334+
335+
def test_parameter_not_a_multiple_of_step__ok(self):
336+
"""Should not raise an error if the relevant ticks parameter is a multiple of 'step'."""
337+
# RI003 RP001
338+
md = """
339+
| survey |
302340
| | type | name | label | parameters |
303-
| | range | q1 | Q1 | start=-3 end=3 step=2 {name}={value} |
341+
| | range | q1 | Q1 | start=-1 end=1 step=1 {name}={value} |
304342
"""
305-
params = ("tick_interval", "placeholder")
306-
cases = ("3", "-3")
307-
for name in params:
343+
params = (
344+
("tick_interval", "odk:tick-interval"),
345+
("placeholder", "odk:placeholder"),
346+
)
347+
cases = ("1", "-1")
348+
for name, attr in params:
308349
for value in cases:
309-
with self.subTest((name, value)):
350+
with self.subTest((name, attr, value)):
310351
self.assertPyxformXform(
311352
md=md.format(name=name, value=value),
312-
errored=True,
313-
error__contains=[
314-
ErrorCode.RANGE_004.value.format(row=2, name=name)
353+
xml__xpath_match=[
354+
xpq.body_range(
355+
"q1",
356+
{"start": "-1", "end": "1", "step": "1", attr: value},
357+
),
315358
],
316359
)
317360

318-
def test_parameter_not_a_multiple_of_step__ok(self):
361+
def test_parameter_not_a_multiple_of_step_decimal__ok(self):
319362
"""Should not raise an error if the relevant ticks parameter is a multiple of 'step'."""
320363
# RI003 RP001
321364
md = """
322365
| survey |
323366
| | type | name | label | parameters |
324-
| | range | q1 | Q1 | start=-1 end=1 step=1 {name}={value} |
367+
| | range | q1 | Q1 | start=-1 end=1 step=0.1 {name}={value} |
325368
"""
326369
params = (
327370
("tick_interval", "odk:tick-interval"),
328371
("placeholder", "odk:placeholder"),
329372
)
330-
cases = ("1", "-1")
373+
cases = ("0.6", "-0.6")
331374
for name, attr in params:
332375
for value in cases:
333376
with self.subTest((name, attr, value)):
@@ -336,7 +379,7 @@ def test_parameter_not_a_multiple_of_step__ok(self):
336379
xml__xpath_match=[
337380
xpq.body_range(
338381
"q1",
339-
{"start": "-1", "end": "1", "step": "1", attr: value},
382+
{"start": "-1", "end": "1", "step": "0.1", attr: value},
340383
),
341384
],
342385
)
@@ -349,7 +392,7 @@ def test_placeholder_outside_range__error(self):
349392
| | type | name | label | parameters |
350393
| | range | q1 | Q1 | start=3 end=7 step=2 placeholder={value} |
351394
"""
352-
cases = ("2", "8")
395+
cases = ("1", "9")
353396
for value in cases:
354397
with self.subTest(value):
355398
self.assertPyxformXform(
@@ -368,7 +411,7 @@ def test_placeholder_outside_inverted_range__error(self):
368411
| | type | name | label | parameters |
369412
| | range | q1 | Q1 | start=7 end=3 step=2 placeholder={value} |
370413
"""
371-
cases = ("8", "1")
414+
cases = ("9", "1")
372415
for value in cases:
373416
with self.subTest(value):
374417
self.assertPyxformXform(

0 commit comments

Comments
 (0)