Skip to content

Commit

Permalink
Update compare_values to support null values (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
echeng06 authored Nov 19, 2024
1 parent a9e3555 commit 8d23e9e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 11 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Documentation of release versions of `nacc-form-validator`
* Updates `compare_with` to support the `abs` operator
* Updates `compatibility` error message to be more verbose
* Refactors the tests to be more modularized so that they're more manageable
* Refactors logic for `compare_values` by moving it to its own utility method
* Refactors logic for `compare_values` by moving it to its own utility method and allows comparing to null values
* Adds `get_previous_record` method to grab previous record from Datastore, which can grab the previous record or the previous record where a specific field is non-empty
* Updates `_validate_temporalrules` to also support the `ignore_empty` parameter
* Adds support for comparing against the previous record in `compare_with`
Expand Down
25 changes: 18 additions & 7 deletions nacc_form_validator/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,23 @@ def compare_values(comparator: str, value: object, base_value: object) -> bool:
Returns:
bool: True if the formula is satisfied, else False
"""
# test these first as they don't care about null values
if comparator == "==":
return value == base_value

if comparator == "!=":
return value != base_value

# for < and >, follow same convention as jsonlogic for null values
# for >= and <=, allow equality case (both None)
if value is None and base_value is None:
return True if comparator in ["<=", ">="] else False
if value is None:
return True if comparator in ["<", "<="] else False
if base_value is None:
return False if comparator in ["<", "<="] else True

# now try as normal
if comparator == ">=":
return value >= base_value

Expand All @@ -80,10 +97,4 @@ def compare_values(comparator: str, value: object, base_value: object) -> bool:
if comparator == "<":
return value < base_value

if comparator == "==":
return value == base_value

if comparator == "!=":
return value != base_value

return False
raise TypeError(f"Unrecognized comparator: {comparator}")
28 changes: 25 additions & 3 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ def test_compare_values_date_invalid():
assert not compare_values("!=", parser.parse("01/01/2000"), parser.parse("01/01/2000"))

def test_compare_values_type_error():
""" Test comparing two values of incompatible types """
""" Test comparing two values of incompatible types or unrecognized comparator """
with pytest.raises(TypeError) as e:
compare_values("<", None, "hello")
assert str(e.value) == "'<' not supported between instances of 'NoneType' and 'str'"
compare_values("*", 5, 10)
assert str(e.value) == "Unrecognized comparator: *"

with pytest.raises(TypeError) as e:
compare_values("<", 5, parser.parse("01/01/2000"))
Expand All @@ -122,3 +122,25 @@ def test_compare_values_type_error():
with pytest.raises(TypeError) as e:
compare_values("<", "01/01/2000", parser.parse("01/01/2000"))
assert str(e.value) == "'<' not supported between instances of 'str' and 'datetime.datetime'"

def test_compare_values_null_values_valid():
""" Test comparing when at least one of the values is null """
assert compare_values("==", None, None)
assert not compare_values("==", None, 5)
assert not compare_values("!=", None, None)
assert compare_values("!=", 5, None)

assert compare_values("<", None, 5)
assert not compare_values("<", 5, None)
assert not compare_values(">", None, 5)
assert compare_values(">", 5, None)

assert compare_values("<=", None, 5)
assert not compare_values("<=", 5, None)
assert not compare_values(">=", None, 5)
assert compare_values(">=", 5, None)

assert not compare_values("<", None, None)
assert not compare_values(">", None, None)
assert compare_values("<=", None, None)
assert compare_values(">=", None, None)

0 comments on commit 8d23e9e

Please sign in to comment.