From 8d23e9e290e7c1eeac71fdd43a93c7b6d83199a3 Mon Sep 17 00:00:00 2001 From: Emily Cheng Date: Tue, 19 Nov 2024 09:59:55 -0800 Subject: [PATCH] Update compare_values to support null values (#26) --- CHANGELOG.md | 2 +- nacc_form_validator/utils.py | 25 ++++++++++++++++++------- tests/test_utils.py | 28 +++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7afc670..f47f503 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` diff --git a/nacc_form_validator/utils.py b/nacc_form_validator/utils.py index 40924bb..99d4824 100644 --- a/nacc_form_validator/utils.py +++ b/nacc_form_validator/utils.py @@ -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 @@ -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}") diff --git a/tests/test_utils.py b/tests/test_utils.py index 63de2bc..f62187a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -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")) @@ -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)