From 85c0374be03deedcf4d118e5f0ef0661ee4daeaf Mon Sep 17 00:00:00 2001 From: Dylan Mordaunt <15080672+edithatogo@users.noreply.github.com> Date: Thu, 9 Jul 2026 11:25:25 +1000 Subject: [PATCH 1/3] Add explicit input / value-state provenance helpers Expose Holder.is_input, Simulation.is_input, and Simulation.get_value_state so screener-style callers can distinguish intentional zeros from omitted inputs. Calculation defaults are unchanged; this is an opt-in query API over the existing _user_input_keys tracking. Closes PolicyEngine/policyengine-core#513 --- .../explicit-input-value-state.added.md | 1 + policyengine_core/holders/holder.py | 35 +++++++++++++ policyengine_core/simulations/simulation.py | 26 ++++++++++ tests/core/test_holders.py | 49 +++++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 changelog.d/explicit-input-value-state.added.md diff --git a/changelog.d/explicit-input-value-state.added.md b/changelog.d/explicit-input-value-state.added.md new file mode 100644 index 00000000..9d9960ff --- /dev/null +++ b/changelog.d/explicit-input-value-state.added.md @@ -0,0 +1 @@ +Added `Holder.is_input`, `Simulation.is_input`, and `Simulation.get_value_state` so callers can distinguish explicit inputs (including zeros) from omitted defaults without changing calculation behavior. diff --git a/policyengine_core/holders/holder.py b/policyengine_core/holders/holder.py index f6be8d04..cdfe4dda 100644 --- a/policyengine_core/holders/holder.py +++ b/policyengine_core/holders/holder.py @@ -142,6 +142,41 @@ def get_array(self, period: Period, branch_name: str = "default") -> ArrayLike: if default_value is not None: return default_value + def is_input(self, period: Period, branch_name: str = "default") -> bool: + """Return whether this variable was explicitly set as an input. + + Distinguishes user-provided values (including explicit zeros) from + values that only exist because a formula ran or a default was applied. + Tracking uses the simulation's existing ``_user_input_keys`` set, which + :meth:`set_input` already maintains; formula cache writes via + :meth:`put_in_cache` are not treated as inputs. + """ + simulation = getattr(self, "simulation", None) + if simulation is None: + return False + user_input_keys = getattr(simulation, "_user_input_keys", None) + if not user_input_keys: + return False + + period = periods.period(period) + if (self.variable.name, branch_name, period) in user_input_keys: + return True + + # Nested branches inherit user inputs from ancestors and default. + if branch_name != "default": + parent = getattr(simulation, "parent_branch", None) + while parent is not None: + if ( + self.variable.name, + parent.branch_name, + period, + ) in user_input_keys: + return True + parent = getattr(parent, "parent_branch", None) + if (self.variable.name, "default", period) in user_input_keys: + return True + return False + def get_memory_usage(self) -> dict: """ Get data about the virtual memory usage of the holder. diff --git a/policyengine_core/simulations/simulation.py b/policyengine_core/simulations/simulation.py index b7f52591..407938c6 100644 --- a/policyengine_core/simulations/simulation.py +++ b/policyengine_core/simulations/simulation.py @@ -1240,6 +1240,32 @@ def get_array(self, variable_name: str, period: Period) -> ArrayLike: period = periods.period(period) return self.get_holder(variable_name).get_array(period, self.branch_name) + def is_input(self, variable_name: str, period: Any) -> bool: + """Return whether ``variable_name`` was explicitly set as an input. + + This does not change calculation defaults: omitted numeric inputs still + default to zero (or the variable's ``default_value``) during formulas. + Use this helper when screener-style flows need to tell an intentional + zero apart from a field the user never provided. + + :returns: ``True`` if :meth:`Holder.set_input` recorded the key for the + current branch (or an ancestor branch), else ``False``. + """ + if period is not None and not isinstance(period, Period): + period = periods.period(period) + return self.get_holder(variable_name).is_input(period, self.branch_name) + + def get_value_state(self, variable_name: str, period: Any) -> str: + """Return input provenance for ``variable_name`` at ``period``. + + :returns: ``"explicit"`` when the value was set via :meth:`set_input` + / situation inputs; ``"default"`` when it was not. Calculated + (formula-filled) values are also ``"default"`` from an + input-provenance perspective until a richer value-state model is + introduced. + """ + return "explicit" if self.is_input(variable_name, period) else "default" + def get_holder(self, variable_name: str) -> Holder: """ Get the :obj:`.Holder` associated with the variable ``variable_name`` for the simulation diff --git a/tests/core/test_holders.py b/tests/core/test_holders.py index 59b77c47..9c84e9d9 100644 --- a/tests/core/test_holders.py +++ b/tests/core/test_holders.py @@ -279,3 +279,52 @@ def test__given_nan_cache_value__then_put_in_cache_keeps_internal_write_allowed( salary_holder.put_in_cache(numpy.asarray([numpy.nan]), period) assert numpy.isnan(salary_holder.get_array(period)).all() + + +def test_is_input_distinguishes_explicit_zero_from_missing(single): + simulation = single + salary_holder = simulation.person.get_holder("salary") + + # Never provided: not an input, value state is default. + assert not salary_holder.is_input(period) + assert not simulation.is_input("salary", period) + assert simulation.get_value_state("salary", period) == "default" + + # Explicit zero is still an input (distinct from "missing"). + salary_holder.set_input(period, numpy.asarray([0])) + assert salary_holder.is_input(period) + assert simulation.is_input("salary", period) + assert simulation.get_value_state("salary", period) == "explicit" + assert salary_holder.get_array(period) == numpy.asarray([0]) + + +def test_put_in_cache_is_not_user_input(single): + simulation = single + salary_holder = simulation.person.get_holder("salary") + + salary_holder.put_in_cache(numpy.asarray([1000.0]), period) + + assert salary_holder.get_array(period) is not None + assert not salary_holder.is_input(period) + assert simulation.get_value_state("salary", period) == "default" + + +def test_situation_inputs_are_marked_explicit(tax_benefit_system): + situation = { + "persons": { + "Alicia": { + "salary": { + "2017-12": 1500, + } + } + }, + "households": { + "_": { + "parents": ["Alicia"], + } + }, + } + simulation = SimulationBuilder().build_from_entities(tax_benefit_system, situation) + assert simulation.is_input("salary", "2017-12") + assert simulation.get_value_state("salary", "2017-12") == "explicit" + assert not simulation.is_input("salary", "2017-11") From 11584653a28756fbce51e36295e8ad37dcb5f5e5 Mon Sep 17 00:00:00 2001 From: Dylan Mordaunt <15080672+edithatogo@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:51:05 +1000 Subject: [PATCH 2/3] test: cover Holder.is_input edge cases for missingness helpers Exercise unbound holders, empty user-input keys, and parent-branch inheritance so Codecov patch coverage clears on PR #516. Co-authored-by: Cursor --- tests/core/test_holders.py | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/core/test_holders.py b/tests/core/test_holders.py index 9c84e9d9..f7fe0534 100644 --- a/tests/core/test_holders.py +++ b/tests/core/test_holders.py @@ -328,3 +328,49 @@ def test_situation_inputs_are_marked_explicit(tax_benefit_system): assert simulation.is_input("salary", "2017-12") assert simulation.get_value_state("salary", "2017-12") == "explicit" assert not simulation.is_input("salary", "2017-11") + + +def test_is_input_false_without_simulation_binding(single): + simulation = single + salary_holder = simulation.person.get_holder("salary") + salary_holder.set_input(period, numpy.asarray([0])) + assert salary_holder.is_input(period) + + salary_holder.simulation = None + assert not salary_holder.is_input(period) + + +def test_is_input_false_when_user_input_keys_missing(single): + simulation = single + salary_holder = simulation.person.get_holder("salary") + salary_holder.set_input(period, numpy.asarray([25])) + assert salary_holder.is_input(period) + + simulation._user_input_keys = set() + assert not salary_holder.is_input(period) + assert simulation.get_value_state("salary", period) == "default" + + +def test_is_input_inherits_from_parent_branch(tax_benefit_system): + situation = { + "persons": { + "Alicia": { + "salary": { + "2017-12": 1500, + } + } + }, + "households": { + "_": { + "parents": ["Alicia"], + } + }, + } + simulation = SimulationBuilder().build_from_entities(tax_benefit_system, situation) + child = simulation.get_branch("reform") + salary_holder = child.person.get_holder("salary") + + assert child.branch_name == "reform" + assert salary_holder.is_input("2017-12") + assert child.is_input("salary", "2017-12") + assert child.get_value_state("salary", "2017-12") == "explicit" From 5693dca6da2226fb85c674b30ecfa39bfc3bf38b Mon Sep 17 00:00:00 2001 From: Dylan Mordaunt <15080672+edithatogo@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:04:59 +1000 Subject: [PATCH 3/3] refactor(holders): reuse visible-branch walk for is_input Delegate current-branch inheritance to Simulation._get_visible_branch_names, document ETERNITY period caveat, and name planned value-state vocabulary. Co-authored-by: Cursor --- policyengine_core/holders/holder.py | 34 ++++++++++++--------- policyengine_core/simulations/simulation.py | 16 +++++++--- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/policyengine_core/holders/holder.py b/policyengine_core/holders/holder.py index cdfe4dda..c4a5606e 100644 --- a/policyengine_core/holders/holder.py +++ b/policyengine_core/holders/holder.py @@ -150,6 +150,18 @@ def is_input(self, period: Period, branch_name: str = "default") -> bool: Tracking uses the simulation's existing ``_user_input_keys`` set, which :meth:`set_input` already maintains; formula cache writes via :meth:`put_in_cache` are not treated as inputs. + + When ``branch_name`` matches the simulation's current branch, inheritance + uses :meth:`Simulation._get_visible_branch_names` (same walk as exportable + input periods). Queries for a different branch only check that branch's + exact key — they do not walk ``parent_branch``, which is only meaningful + relative to the simulation's current branch. + + Note: variables with ``definition_period == ETERNITY`` store inputs under + the period key recorded by :meth:`set_input` (often the ETERNITY period + itself). Callers that pass a concrete month/year may miss that key until + a dedicated ETERNITY canonicalization is added; screener monetary inputs + are typically MONTH/YEAR and are unaffected. """ simulation = getattr(self, "simulation", None) if simulation is None: @@ -159,23 +171,15 @@ def is_input(self, period: Period, branch_name: str = "default") -> bool: return False period = periods.period(period) - if (self.variable.name, branch_name, period) in user_input_keys: - return True + variable_name = self.variable.name - # Nested branches inherit user inputs from ancestors and default. - if branch_name != "default": - parent = getattr(simulation, "parent_branch", None) - while parent is not None: - if ( - self.variable.name, - parent.branch_name, - period, - ) in user_input_keys: + if branch_name == getattr(simulation, "branch_name", "default"): + for visible_branch in simulation._get_visible_branch_names(): + if (variable_name, visible_branch, period) in user_input_keys: return True - parent = getattr(parent, "parent_branch", None) - if (self.variable.name, "default", period) in user_input_keys: - return True - return False + return False + + return (variable_name, branch_name, period) in user_input_keys def get_memory_usage(self) -> dict: """ diff --git a/policyengine_core/simulations/simulation.py b/policyengine_core/simulations/simulation.py index 407938c6..1574b822 100644 --- a/policyengine_core/simulations/simulation.py +++ b/policyengine_core/simulations/simulation.py @@ -1258,11 +1258,17 @@ def is_input(self, variable_name: str, period: Any) -> bool: def get_value_state(self, variable_name: str, period: Any) -> str: """Return input provenance for ``variable_name`` at ``period``. - :returns: ``"explicit"`` when the value was set via :meth:`set_input` - / situation inputs; ``"default"`` when it was not. Calculated - (formula-filled) values are also ``"default"`` from an - input-provenance perspective until a richer value-state model is - introduced. + Current vocabulary (stable for callers): + + - ``"explicit"`` — value was set via :meth:`set_input` / situation inputs + - ``"default"`` — not recorded as a user input (includes omitted fields + that still default to zero in formulas, and formula-filled / cached + values from an input-provenance perspective) + + Planned future states (not returned yet): ``"computed"`` for + formula-filled values, and ``"unknown"`` when provenance cannot be + determined. Until those land, treat anything non-explicit as + ``"default"``. """ return "explicit" if self.is_input(variable_name, period) else "default"