From c20c3645ddf7ad786b7dafd7033995bb077e242a Mon Sep 17 00:00:00 2001 From: Abhishek Singh <31144719+abhcs@users.noreply.github.com> Date: Tue, 16 Jan 2024 14:30:39 -0600 Subject: [PATCH 1/4] feat: raise exception if variable lacks a label or if it has a formula and an add/subtract --- policyengine_core/variables/variable.py | 11 +++++++++++ tests/core/parameters/operations/test_nesting.py | 3 +++ tests/core/test_calculate_output.py | 3 +++ tests/core/test_cycles.py | 8 ++++++++ tests/core/test_formulas.py | 6 ++++++ tests/core/test_projectors.py | 7 +++++++ tests/core/test_simulation_builder.py | 3 +++ tests/core/variables/test_annualize.py | 1 + tests/fixtures/variables.py | 1 + 9 files changed, 43 insertions(+) diff --git a/policyengine_core/variables/variable.py b/policyengine_core/variables/variable.py index 4674d26c1..80cb74a1b 100644 --- a/policyengine_core/variables/variable.py +++ b/policyengine_core/variables/variable.py @@ -187,6 +187,12 @@ def __init__(self, baseline_variable=None): self.label = self.set( attr, "label", allowed_type=str, setter=self.set_label ) + + if self.label is None: + raise ValueError( + 'Variable "{}" has no label'.format(self.name) + ) + self.end = self.set(attr, "end", allowed_type=str, setter=self.set_end) self.reference = self.set(attr, "reference", setter=self.set_reference) self.cerfa_field = self.set( @@ -285,6 +291,11 @@ def __init__(self, baseline_variable=None): self.name, ", ".join(sorted(unexpected_attrs.keys())) ) ) + + if len(self.formulas) != 0 and (self.adds is not None or self.subtracts is not None): + raise ValueError( + 'Variable "{}" has a formula and an add or subtract'.format(self.name) + ) self.is_neutralized = False diff --git a/tests/core/parameters/operations/test_nesting.py b/tests/core/parameters/operations/test_nesting.py index 3ba110d01..a4a808798 100644 --- a/tests/core/parameters/operations/test_nesting.py +++ b/tests/core/parameters/operations/test_nesting.py @@ -39,6 +39,7 @@ class country(Variable): definition_period = ETERNITY possible_values = Country default_value = Country.ENGLAND + label = "country" class Region(Enum): NORTH_EAST = "North East" @@ -60,11 +61,13 @@ class region(Variable): definition_period = ETERNITY possible_values = Region default_value = Region.NORTH_EAST + label = "region" class family_size(Variable): value_type = int entity = Person definition_period = ETERNITY + label = "family size" from policyengine_core.parameters import homogenize_parameter_structures from policyengine_core.taxbenefitsystems import TaxBenefitSystem diff --git a/tests/core/test_calculate_output.py b/tests/core/test_calculate_output.py index b849a9d5f..7211afb58 100644 --- a/tests/core/test_calculate_output.py +++ b/tests/core/test_calculate_output.py @@ -10,6 +10,7 @@ class simple_variable(Variable): entity = entities.Person definition_period = periods.MONTH value_type = int + label = "simple variable" class variable_with_calculate_output_add(Variable): @@ -17,6 +18,7 @@ class variable_with_calculate_output_add(Variable): definition_period = periods.MONTH value_type = int calculate_output = simulations.calculate_output_add + label = "variable with calculate_output_add" class variable_with_calculate_output_divide(Variable): @@ -24,6 +26,7 @@ class variable_with_calculate_output_divide(Variable): definition_period = periods.YEAR value_type = int calculate_output = simulations.calculate_output_divide + label = "variable with calculate_output_divide" @pytest.fixture(scope="module", autouse=True) diff --git a/tests/core/test_cycles.py b/tests/core/test_cycles.py index e65e699df..cee41bbac 100644 --- a/tests/core/test_cycles.py +++ b/tests/core/test_cycles.py @@ -22,6 +22,7 @@ class variable1(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 1" def formula(person, period): return person("variable2", period) @@ -31,6 +32,7 @@ class variable2(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 2" def formula(person, period): return person("variable1", period) @@ -41,6 +43,7 @@ class variable3(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 3" def formula(person, period): return person("variable4", period.last_month) @@ -50,6 +53,7 @@ class variable4(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 4" def formula(person, period): return person("variable3", period) @@ -61,6 +65,7 @@ class variable5(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 5" def formula(person, period): variable6 = person("variable6", period.last_month) @@ -71,6 +76,7 @@ class variable6(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 6" def formula(person, period): variable5 = person("variable5", period) @@ -81,6 +87,7 @@ class variable7(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "variable 7" def formula(person, period): variable5 = person("variable5", period) @@ -92,6 +99,7 @@ class cotisation(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "cotisation" def formula(person, period): if period.start.month == 12: diff --git a/tests/core/test_formulas.py b/tests/core/test_formulas.py index 08f1c1603..3504283a3 100644 --- a/tests/core/test_formulas.py +++ b/tests/core/test_formulas.py @@ -11,6 +11,7 @@ class choice(Variable): value_type = int entity = entities.Person definition_period = periods.MONTH + label = "choice" class uses_multiplication(Variable): @@ -18,6 +19,7 @@ class uses_multiplication(Variable): entity = entities.Person label = "Variable with formula that uses multiplication" definition_period = periods.MONTH + label = "uses multiplication" def formula(person, period): choice = person("choice", period) @@ -30,6 +32,7 @@ class returns_scalar(Variable): entity = entities.Person label = "Variable with formula that returns a scalar value" definition_period = periods.MONTH + label = "returns scalar" def formula(person, period): return 666 @@ -40,6 +43,7 @@ class uses_switch(Variable): entity = entities.Person label = "Variable with formula that uses switch" definition_period = periods.MONTH + label = "uses switch" def formula(person, period): choice = person("choice", period) @@ -154,11 +158,13 @@ class household_level_variable(Variable): value_type = int entity = household_entity definition_period = ETERNITY + label = "household level variable" class projected_family_level_variable(Variable): value_type = int entity = family_entity definition_period = ETERNITY + label = "projected family level variable" def formula(family, period): return family.household("household_level_variable", period) diff --git a/tests/core/test_projectors.py b/tests/core/test_projectors.py index 765a608b7..a2e3444af 100644 --- a/tests/core/test_projectors.py +++ b/tests/core/test_projectors.py @@ -137,6 +137,7 @@ class household_enum_variable(Variable): default_value = enum.FIRST_OPTION entity = household definition_period = ETERNITY + label = "household enum variable" class projected_enum_variable(Variable): value_type = Enum @@ -144,6 +145,7 @@ class projected_enum_variable(Variable): default_value = enum.FIRST_OPTION entity = person definition_period = ETERNITY + label = "projected enum variable" def formula(person, period): return person.household("household_enum_variable", period) @@ -210,6 +212,7 @@ class household_projected_variable(Variable): default_value = enum.FIRST_OPTION entity = household definition_period = ETERNITY + label = "household projected variable" def formula(household, period): return household.value_from_first_person( @@ -222,6 +225,7 @@ class person_enum_variable(Variable): default_value = enum.FIRST_OPTION entity = person definition_period = ETERNITY + label = "person enum variable" system.add_variables(household_projected_variable, person_enum_variable) @@ -303,6 +307,7 @@ class household_level_variable(Variable): default_value = enum.FIRST_OPTION entity = household_entity definition_period = ETERNITY + label = "household level variable" class projected_family_level_variable(Variable): value_type = Enum @@ -310,6 +315,7 @@ class projected_family_level_variable(Variable): default_value = enum.FIRST_OPTION entity = family_entity definition_period = ETERNITY + label = "projected family level variable" def formula(family, period): return family.household("household_level_variable", period) @@ -318,6 +324,7 @@ class decoded_projected_family_level_variable(Variable): value_type = str entity = family_entity definition_period = ETERNITY + label = "decoded projected family level variable" def formula(family, period): return family.household( diff --git a/tests/core/test_simulation_builder.py b/tests/core/test_simulation_builder.py index 4ed4fbe08..d9c064be7 100644 --- a/tests/core/test_simulation_builder.py +++ b/tests/core/test_simulation_builder.py @@ -19,6 +19,7 @@ class intvar(Variable): definition_period = periods.ETERNITY value_type = int entity = persons + label = "int variable" def __init__(self): super().__init__() @@ -32,6 +33,7 @@ class datevar(Variable): definition_period = periods.ETERNITY value_type = datetime.date entity = persons + label = "date variable" def __init__(self): super().__init__() @@ -50,6 +52,7 @@ class TestEnum(Variable): set_input = None possible_values = Enum("foo", "bar") name = "enum" + label = "enum variable" def __init__(self): pass diff --git a/tests/core/variables/test_annualize.py b/tests/core/variables/test_annualize.py index a36b8d05d..61f8168b8 100644 --- a/tests/core/variables/test_annualize.py +++ b/tests/core/variables/test_annualize.py @@ -15,6 +15,7 @@ class monthly_variable(Variable): value_type = int entity = Person definition_period = MONTH + label = "monthly variable" def formula(person, period, parameters): variable.calculation_count += 1 diff --git a/tests/fixtures/variables.py b/tests/fixtures/variables.py index 79d28d146..600107515 100644 --- a/tests/fixtures/variables.py +++ b/tests/fixtures/variables.py @@ -6,6 +6,7 @@ class TestVariable(Variable): definition_period = periods.ETERNITY value_type = float + label = "test variable" def __init__(self, entity): self.__class__.entity = entity From c5b4a864770dec32aa5e922fa22786824eebe52a Mon Sep 17 00:00:00 2001 From: Abhishek Singh <31144719+abhcs@users.noreply.github.com> Date: Tue, 16 Jan 2024 19:03:14 -0600 Subject: [PATCH 2/4] feat: add tests for well-formed variables --- policyengine_core/variables/variable.py | 4 +- tests/core/variables/test_variables.py | 84 +++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/policyengine_core/variables/variable.py b/policyengine_core/variables/variable.py index 80cb74a1b..12daa0d51 100644 --- a/policyengine_core/variables/variable.py +++ b/policyengine_core/variables/variable.py @@ -190,7 +190,7 @@ def __init__(self, baseline_variable=None): if self.label is None: raise ValueError( - 'Variable "{}" has no label'.format(self.name) + 'Variable "{name}" has no label'.format(name=self.name) ) self.end = self.set(attr, "end", allowed_type=str, setter=self.set_end) @@ -294,7 +294,7 @@ def __init__(self, baseline_variable=None): if len(self.formulas) != 0 and (self.adds is not None or self.subtracts is not None): raise ValueError( - 'Variable "{}" has a formula and an add or subtract'.format(self.name) + 'Variable "{name}" has a formula and an add or subtract'.format(name=self.name) ) self.is_neutralized = False diff --git a/tests/core/variables/test_variables.py b/tests/core/variables/test_variables.py index aad2fa1db..99e667a37 100644 --- a/tests/core/variables/test_variables.py +++ b/tests/core/variables/test_variables.py @@ -612,3 +612,87 @@ class variable_with_strange_attr(Variable): with raises(ValueError): tax_benefit_system.add_variable(variable_with_strange_attr) + +class variable__one_formula_one_add(Variable): + value_type = int + entity = Person + definition_period = MONTH + label = "Variable with one formula and one add." + adds = ["pass"] + + def formula(): + pass + +def test_one_formula_one_add(): + check_error_at_add_variable( + tax_benefit_system, + variable__one_formula_one_add, + 'Variable "{name}" has a formula and an add or subtract'.format(name="variable__one_formula_one_add"), + ) + +class variable__one_formula_one_subtract(Variable): + value_type = int + entity = Person + definition_period = MONTH + label = "Variable with one formula and one subtract." + adds = ["pass"] + + def formula(): + pass + +def test_one_formula_one_subtract(): + check_error_at_add_variable( + tax_benefit_system, + variable__one_formula_one_subtract, + 'Variable "{name}" has a formula and an add or subtract'.format(name="variable__one_formula_one_subtract"), + ) + +class variable__one_formula(Variable): + value_type = int + entity = Person + definition_period = MONTH + label = "Variable with one formula." + + def formula(): + pass + +def test_one_formula(): + tax_benefit_system.add_variable(variable__one_formula) + variable = tax_benefit_system.variables["variable__one_formula"] + assert len(variable.formulas) + +class variable__one_add(Variable): + value_type = int + entity = Person + definition_period = MONTH + label = "Variable with one add." + adds = ["pass"] + +def test_one_add(): + tax_benefit_system.add_variable(variable__one_add) + variable = tax_benefit_system.variables["variable__one_add"] + assert len(variable.adds) + +class variable__one_subtract(Variable): + value_type = int + entity = Person + definition_period = MONTH + label = "Variable with one subtract." + subtracts = ["pass"] + +def test_one_subtract(): + tax_benefit_system.add_variable(variable__one_subtract) + variable = tax_benefit_system.variables["variable__one_subtract"] + assert len(variable.subtracts) + +class variable__no_label(Variable): + value_type = int + entity = Person + definition_period = MONTH + +def test_no_label(): + check_error_at_add_variable( + tax_benefit_system, + variable__no_label, + 'Variable "{name}" has no label'.format(name="variable__no_label") + ) \ No newline at end of file From dc45ff4dfc41c1da05ab6eb58df0b9b08f171f19 Mon Sep 17 00:00:00 2001 From: Abhishek Singh <31144719+abhcs@users.noreply.github.com> Date: Tue, 16 Jan 2024 19:12:17 -0600 Subject: [PATCH 3/4] chore: format --- policyengine_core/variables/variable.py | 12 ++++++++---- tests/core/variables/test_variables.py | 24 ++++++++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/policyengine_core/variables/variable.py b/policyengine_core/variables/variable.py index 12daa0d51..8fa44435e 100644 --- a/policyengine_core/variables/variable.py +++ b/policyengine_core/variables/variable.py @@ -192,7 +192,7 @@ def __init__(self, baseline_variable=None): raise ValueError( 'Variable "{name}" has no label'.format(name=self.name) ) - + self.end = self.set(attr, "end", allowed_type=str, setter=self.set_end) self.reference = self.set(attr, "reference", setter=self.set_reference) self.cerfa_field = self.set( @@ -291,10 +291,14 @@ def __init__(self, baseline_variable=None): self.name, ", ".join(sorted(unexpected_attrs.keys())) ) ) - - if len(self.formulas) != 0 and (self.adds is not None or self.subtracts is not None): + + if len(self.formulas) != 0 and ( + self.adds is not None or self.subtracts is not None + ): raise ValueError( - 'Variable "{name}" has a formula and an add or subtract'.format(name=self.name) + 'Variable "{name}" has a formula and an add or subtract'.format( + name=self.name + ) ) self.is_neutralized = False diff --git a/tests/core/variables/test_variables.py b/tests/core/variables/test_variables.py index 99e667a37..12e5e8acb 100644 --- a/tests/core/variables/test_variables.py +++ b/tests/core/variables/test_variables.py @@ -613,6 +613,7 @@ class variable_with_strange_attr(Variable): with raises(ValueError): tax_benefit_system.add_variable(variable_with_strange_attr) + class variable__one_formula_one_add(Variable): value_type = int entity = Person @@ -623,13 +624,17 @@ class variable__one_formula_one_add(Variable): def formula(): pass + def test_one_formula_one_add(): check_error_at_add_variable( tax_benefit_system, variable__one_formula_one_add, - 'Variable "{name}" has a formula and an add or subtract'.format(name="variable__one_formula_one_add"), + 'Variable "{name}" has a formula and an add or subtract'.format( + name="variable__one_formula_one_add" + ), ) + class variable__one_formula_one_subtract(Variable): value_type = int entity = Person @@ -640,13 +645,17 @@ class variable__one_formula_one_subtract(Variable): def formula(): pass + def test_one_formula_one_subtract(): check_error_at_add_variable( tax_benefit_system, variable__one_formula_one_subtract, - 'Variable "{name}" has a formula and an add or subtract'.format(name="variable__one_formula_one_subtract"), + 'Variable "{name}" has a formula and an add or subtract'.format( + name="variable__one_formula_one_subtract" + ), ) + class variable__one_formula(Variable): value_type = int entity = Person @@ -656,11 +665,13 @@ class variable__one_formula(Variable): def formula(): pass + def test_one_formula(): tax_benefit_system.add_variable(variable__one_formula) variable = tax_benefit_system.variables["variable__one_formula"] assert len(variable.formulas) + class variable__one_add(Variable): value_type = int entity = Person @@ -668,11 +679,13 @@ class variable__one_add(Variable): label = "Variable with one add." adds = ["pass"] + def test_one_add(): tax_benefit_system.add_variable(variable__one_add) variable = tax_benefit_system.variables["variable__one_add"] assert len(variable.adds) + class variable__one_subtract(Variable): value_type = int entity = Person @@ -680,19 +693,22 @@ class variable__one_subtract(Variable): label = "Variable with one subtract." subtracts = ["pass"] + def test_one_subtract(): tax_benefit_system.add_variable(variable__one_subtract) variable = tax_benefit_system.variables["variable__one_subtract"] assert len(variable.subtracts) + class variable__no_label(Variable): value_type = int entity = Person definition_period = MONTH + def test_no_label(): check_error_at_add_variable( tax_benefit_system, variable__no_label, - 'Variable "{name}" has no label'.format(name="variable__no_label") - ) \ No newline at end of file + 'Variable "{name}" has no label'.format(name="variable__no_label"), + ) From 0d562eb2f6735b86739db33564c7d086ba6d55af Mon Sep 17 00:00:00 2001 From: Abhishek Singh <31144719+abhcs@users.noreply.github.com> Date: Tue, 6 Feb 2024 09:33:32 -0600 Subject: [PATCH 4/4] chore: log change description --- changelog_entry.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changelog_entry.yaml b/changelog_entry.yaml index e69de29bb..f80112b67 100644 --- a/changelog_entry.yaml +++ b/changelog_entry.yaml @@ -0,0 +1,5 @@ +- bump: minor + changes: + changed: + - A variable must have a label. + - If a variable has formulas then it must not have adds/subtracts, and vice versa. \ No newline at end of file