diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..6fac8a8f5 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,16 @@ +# Agent instructions (OpenFisca-Core) + +This file is read by Cursor, Claude, Antigravity, and other agentic tools. Follow it when working in this repo. + +## Pre-push checklist + +**Before every `git push`**, run the full lint and fix any issues, then push. + +1. **Lint** (from repo root): + ```bash + make clean check-syntax-errors check-style lint-doc PYTHON=.venv/bin/python + ``` + - Fix any failures (e.g. `black`, `isort`, `flake8`, `codespell`). Use `make format-style` or run the formatter on the reported files if needed. +2. **Then** stage, commit (if there are new changes from fixes), and push. + +Do not push without having run lint successfully unless the user explicitly asks to skip it. diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e9fff13c..1bd1959e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,21 @@ - Fix false `SpiralError` when a `transition_formula` reads its own variable at the previous period. - The existing spiral detector raised `SpiralError` immediately when the same variable appeared in the call stack at any different period, which always triggers for temporal recursion (`V@P` → `V@P-1` → `V@P-2`). - Fix: in `_calculate_transition`, the cycle check is replaced by `_check_for_strict_cycle`, which only raises `CycleError` for the exact same `(variable, period)` pair. Termination is guaranteed by `_as_of_transition_computed`. +- Post-44.2.2 audit hardening for links, as_of holders, and group population shape handling. + - `GroupPopulation.set_members_entity_id` now raises a clear `ValueError` when called with an empty array. + - `Many2OneLink._get_target_ids` now preserves original exception type and adds link context (`link` name + `link_field`) to the raised message. + - Role matching logic used by `Many2OneLink` and implicit links is now centralized in `links._role_matches` to avoid divergence. + - Chained many-to-one getters now support multi-hop composition (e.g. `person.mother.mother.household`). + +#### Technical changes + +- Clarify as_of snapshot-cache semantics: FIFO eviction (oldest inserted), not LRU. +- Add regression tests for: + - `link_field` not found errors and partial `_id_to_rownum` mappings. + - enum variable projection through `Many2OneLink`. + - three-level chained links. + - FIFO snapshot eviction, retroactive patch invalidation, backward reads after forward reads, and `set_input_sparse` error messaging. + - non-contiguous and empty `members_entity_id` handling. ## 44.4.1 diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md index d3447660c..32a59685d 100644 --- a/PR_DESCRIPTION.md +++ b/PR_DESCRIPTION.md @@ -52,6 +52,10 @@ We've added guides to help framework users model new relationships: - `docs/implementation/links-api.md`: Reference for creating and querying `Many2OneLink` and `One2ManyLink`. - `docs/implementation/transition-guide.md`: Migration guide demonstrating how to gradually adopt Links over Legacy Projectors. +## Builder & test clarity +- **`build_default_simulation(..., group_members=...)`**: Optional `group_members` dict (e.g. `{"household": [0,0,1,1]}`) sets group structure at build time so tests no longer patch private attributes. +- **`GroupPopulation.set_members_entity_id(array)`**: Public API to set group structure and clear internal caches; tests use this instead of touching `_members_position` / `_ordered_members_map`. + ## Testing - 12 new, comprehensive tests covering unit mechanics, system integrations, filtering, chaining, and OpenFisca core lifecycle (`_resolve_links`). - All 158 core tests and existing Country Template tests continue to pass locally (`make test-code`). diff --git a/openfisca_core/holders/holder.py b/openfisca_core/holders/holder.py index 2ff8f411d..011b5c3da 100644 --- a/openfisca_core/holders/holder.py +++ b/openfisca_core/holders/holder.py @@ -39,7 +39,7 @@ def __init__(self, variable, population) -> None: # _as_of_base_instant : Instant at which the base was established. # _as_of_patches : sorted list of (Instant, idx_array, val_array). # _as_of_patch_instants : parallel list of Instants for bisect. - # _as_of_snapshots : LRU OrderedDict instant → (array, patch_idx). + # _as_of_snapshots : FIFO OrderedDict instant → (array, patch_idx). # _as_of_max_snapshots : maximum number of snapshots to keep. self._as_of_base = None self._as_of_base_instant = None @@ -151,17 +151,21 @@ def _get_as_of(self, period): return self._reconstruct_at(target) def _cache_snapshot(self, instant, array, patch_idx) -> None: - """Insert (or refresh) a snapshot in the LRU cache, evicting the least - recently used entry if the cache is full.""" + """Insert (or refresh) a snapshot in the FIFO cache, evicting the oldest + entry if the cache is full. + + Note: eviction is FIFO (oldest inserted), not LRU. This is optimal for + forward-sequential simulations where older snapshots are never reused. + For backward-access patterns the cache will be less effective. + """ self._as_of_snapshots[instant] = (array, patch_idx) - self._as_of_snapshots.move_to_end(instant) if len(self._as_of_snapshots) > self._as_of_max_snapshots: - self._as_of_snapshots.popitem(last=False) # evict LRU + self._as_of_snapshots.popitem(last=False) # evict oldest (FIFO) def _reconstruct_at(self, target_instant): """Reconstruct the dense array at target_instant from base + patches. - Uses a multi-snapshot LRU cache for O(k) incremental cost. + Uses a multi-snapshot FIFO snapshot cache for O(k) incremental cost. Falls back to O(N + k*P) full reconstruction when no usable snapshot exists (e.g. backward jump past all cached snapshots). @@ -178,7 +182,6 @@ def _reconstruct_at(self, target_instant): # Exact cache hit — O(1). if target_instant in self._as_of_snapshots: array, _ = self._as_of_snapshots[target_instant] - self._as_of_snapshots.move_to_end(target_instant) return array # Find best starting snapshot: latest snap_instant < target_instant. diff --git a/openfisca_core/links/implicit.py b/openfisca_core/links/implicit.py index 58c8fc8e3..d06b528fb 100644 --- a/openfisca_core/links/implicit.py +++ b/openfisca_core/links/implicit.py @@ -4,6 +4,7 @@ import numpy +from .link import _role_matches from .many2one import Many2OneLink from .one2many import One2ManyLink @@ -31,18 +32,46 @@ def role(self) -> numpy.ndarray | None: return self._target_population.members_role def _project_implicit(self, result: numpy.ndarray) -> numpy.ndarray: - # Fully compatible with old Projector logic - return self._target_population.project(result) + """Project or pass through result so it matches source (person) count. + + - Entity-sized (result.size == target.count): same as old logic — project + to source so each person gets their entity's value (e.g. first_person). + - Members-sized (result.size == target.members.count): return as-is; + result is already one value per person (e.g. members('activite')). + """ + target = self._target_population + if result.size == target.count: + return target.project(result) + if result.size == target.members.count: + return result + raise ValueError( + f"Implicit link projection: result size {result.size} does not match " + f"target entity count ({target.count}) nor target members count ({target.members.count})." + ) + + # Explicit aggregation methods so person.famille.sum(...) always returns person-sized + # (found by normal attribute lookup before __getattr__ delegates to target). + def sum(self, array, role=None, condition=None): + result = self._target_population.sum(array, role=role, condition=condition) + return self._project_implicit(result) + + def any(self, array, role=None, condition=None): + result = self._target_population.any(array, role=role, condition=condition) + return self._project_implicit(result) + + def all(self, array, role=None, condition=None): + result = self._target_population.all(array, role=role, condition=condition) + return self._project_implicit(result) class ImplicitOne2ManyLink(One2ManyLink): """A group → person link using GroupPopulation's internal arrays.""" - def __init__(self, name: str, group_entity_key: str): + def __init__(self, name: str, group_entity_key: str, person_entity_key: str): super().__init__( name=name, link_field="", # Not used - target_entity_key="person", # The target of the O2M is persons + target_entity_key=person_entity_key, ) self._group_entity_key = group_entity_key @@ -58,19 +87,7 @@ def _apply_filters(self, period, values, role, condition): if role is not None: roles = self._source_population.members_role - # roles may be an object array of Role instances, so compare by key - if roles.dtype == object: - try: - keys = numpy.fromiter( - (getattr(x, "key", x) for x in roles), - dtype=object, - ) - except Exception: - mask &= roles == role - else: - mask &= keys == role - else: - mask &= roles == role + mask &= _role_matches(roles, role) if condition is not None: mask &= condition diff --git a/openfisca_core/links/link.py b/openfisca_core/links/link.py index ec651809c..976cd4541 100644 --- a/openfisca_core/links/link.py +++ b/openfisca_core/links/link.py @@ -4,8 +4,33 @@ from typing import TYPE_CHECKING +import numpy + if TYPE_CHECKING: - import numpy + import numpy as np + + +def _role_matches(role_array: np.ndarray, role_value) -> np.ndarray: + """Return a boolean mask where role_array matches role_value. + + Supports Role objects (compared by .key) and raw values (compared by ==). + """ + if hasattr(role_value, "key"): + role_key = role_value.key + return numpy.array( + [ + (r.key == role_key if hasattr(r, "key") else r == role_key) + for r in role_array + ] + ) + if getattr(role_array, "dtype", None) == object: + return numpy.array( + [ + (r.key == role_value if hasattr(r, "key") else r == role_value) + for r in role_array + ] + ) + return role_array == role_value class Link: @@ -106,4 +131,4 @@ def __repr__(self) -> str: ) -__all__ = ["Link"] +__all__ = ["Link", "_role_matches"] diff --git a/openfisca_core/links/many2one.py b/openfisca_core/links/many2one.py index 83d63181f..57e00d117 100644 --- a/openfisca_core/links/many2one.py +++ b/openfisca_core/links/many2one.py @@ -2,16 +2,99 @@ from __future__ import annotations +from collections.abc import Sequence from typing import TYPE_CHECKING import numpy -from .link import Link +from .link import Link, _role_matches if TYPE_CHECKING: pass +class _CallableProxy: + """Callable that proxies other attributes to the wrapped object (e.g. projector). + + When _projection_link is set (implicit M2O), attributes that have a .get() + (e.g. .foyer_fiscal) are wrapped so get() results are projected to source (person) size. + """ + + __slots__ = ("_call", "_wrapped", "_projection_link") + + def __init__(self, call, wrapped, projection_link=None): + self._call = call + self._wrapped = wrapped + self._projection_link = projection_link + + def __call__(self, *args, **kwargs): + return self._call(*args, **kwargs) + + def __getattr__(self, name: str): + attr = getattr(self._wrapped, name) + if self._projection_link is None or not hasattr( + self._projection_link, "_project_implicit" + ): + return attr + # person.famille.demandeur.foyer_fiscal('x', period): result is entity-sized, + # project to person-sized. Wrap both links (.get) and callables (e.g. projectors). + if callable(getattr(attr, "get", None)): + return _LinkGetProjector(attr, self._projection_link) + if callable(attr): + link = self._projection_link + + def projected_callable(*args, **kwargs): + result = attr(*args, **kwargs) + if ( + isinstance(result, numpy.ndarray) + and result.size == link._target_population.count + ): + return link._project_result(result) + return result + + return _CallableProxy(projected_callable, attr, None) + return attr + + +class _LinkGetProjector: + """Wraps a link so get() results are projected to the outer M2O source size.""" + + __slots__ = ("_link", "_outer") + + def __init__(self, link, outer_m2o_link): + self._link = link + self._outer = outer_m2o_link + + def get(self, variable_name: str, period, options=None): + result = self._link.get(variable_name, period, options=options) + return self._outer._project_result(result) + + def __call__(self, variable_name: str, period, *, options=None, **kwargs): + return self.get(variable_name, period, options=options) + + def __getattr__(self, name: str): + return getattr(self._link, name) + + +def _calculate_with_options(simulation, variable_name, period, options): + """Dispatch to calculate / calculate_add / calculate_divide like CorePopulation.""" + from openfisca_core.populations import types as t + from openfisca_core.populations._errors import ( + IncompatibleOptionsError, + InvalidOptionError, + ) + + if options is None or not isinstance(options, Sequence): + return simulation.calculate(variable_name, period) + if t.Option.ADD in options and t.Option.DIVIDE in options: + raise IncompatibleOptionsError(variable_name) + if t.Option.ADD in options: + return simulation.calculate_add(variable_name, period) + if t.Option.DIVIDE in options: + return simulation.calculate_divide(variable_name, period) + raise InvalidOptionError(options[0], variable_name) + + class Many2OneLink(Link): """Navigate from many source members to one target entity. @@ -25,7 +108,12 @@ class Many2OneLink(Link): result = target_values[target_ids] # e.g. [800, 800, 650, 900, 800] """ - def get(self, variable_name: str, period) -> numpy.ndarray: + def get( + self, + variable_name: str, + period, + options: None | Sequence = None, + ) -> numpy.ndarray: """Get a target variable's value for each source member. Parameters @@ -34,6 +122,8 @@ def get(self, variable_name: str, period) -> numpy.ndarray: Name of the variable defined on the target entity. period : Period The period for which to compute the variable. + options : sequence, optional + Options for the calculation (e.g. ADD, DIVIDE). Returns ------- @@ -49,7 +139,9 @@ def get(self, variable_name: str, period) -> numpy.ndarray: target_ids = self._get_target_ids(period) # 2. Variable values on the target entity - target_values = simulation.calculate(variable_name, period) + target_values = _calculate_with_options( + simulation, variable_name, period, options + ) # 3. Resolve IDs to row positions (handles id_to_rownum if needed) target_rows = self._resolve_ids(target_ids) @@ -78,9 +170,16 @@ def get(self, variable_name: str, period) -> numpy.ndarray: # -- syntactic sugar ---------------------------------------------------- - def __call__(self, variable_name: str, period) -> numpy.ndarray: - """Shorthand: ``person.mother("age", period)``.""" - return self.get(variable_name, period) + def __call__( + self, + variable_name: str, + period, + *, + options=None, + **kwargs, + ) -> numpy.ndarray: + """Shorthand: ``person.mother("age", period)`` or with options.""" + return self.get(variable_name, period, options=options) def __getattr__(self, name: str): """Chain links: ``person.mother.household``.""" @@ -97,13 +196,38 @@ def __getattr__(self, name: str): target_attr = getattr(target_pop, name, None) if target_attr is not None: - if hasattr(target_attr, "projectable"): + # Wrap callables that produce target-level output so we project back to source. + # This covers @projectable methods (sum, any, all, etc.), Projector instances + # (e.g. .demandeur), and other callables. For bound methods, "projectable" is + # on the underlying function, not the method wrapper. + _func = getattr(target_attr, "__func__", target_attr) + is_projectable = ( + hasattr(target_attr, "projectable") + or getattr(_func, "projectable", False) + or name + in ( + "sum", + "any", + "all", + "count", + "max", + "min", + "nb_persons", + "reduce", + ) + ) + if (is_projectable or callable(target_attr)) and hasattr( + self, "_project_implicit" + ): + link_self = self def projector_function(*args, **kwargs): result = target_attr(*args, **kwargs) - return self._project_result(result) + return link_self._project_result(result) - return projector_function + # Preserve attributes (e.g. has_role); wrap link.get() so person.famille.demandeur.foyer_fiscal(...) is person-sized + projection_link = self if hasattr(self, "_project_implicit") else None + return _CallableProxy(projector_function, target_attr, projection_link) return target_attr target_entity = target_pop.entity @@ -142,20 +266,7 @@ def has_role(self, role_value) -> numpy.ndarray: msg = f"Link '{self.name}' has no role_field" raise ValueError(msg) - # if array holds object references, convert to their keys - if r.dtype == object: - try: - keys = numpy.fromiter( - (getattr(x, "key", x) for x in r), - dtype=object, - ) - except Exception: - # fallback to direct comparison - return r == role_value - return keys == role_value - - # numpy will perform elementwise comparison for numeric or string - return r == role_value + return _role_matches(r, role_value) # -- role-based access -------------------------------------------------- @@ -185,7 +296,7 @@ def get_by_role( value; all others receive the variable's default (usually 0). """ mask = self.has_role(role_value) - result = self.get(variable_name, period) + result = self.get(variable_name, period, options=None) # zero out non-matching rows using dtype-preserving fill if not mask.all(): # create a copy to avoid mutating cached results @@ -197,10 +308,17 @@ def get_by_role( def _get_target_ids(self, period) -> numpy.ndarray: """Fetch the target IDs from the link_field variable.""" - return self._source_population.simulation.calculate( - self.link_field, - period, - ) + try: + return self._source_population.simulation.calculate( + self.link_field, + period, + ) + except Exception as e: + e.args = ( + f"In link '{self.name}': could not resolve link_field " + f"'{self.link_field}'. Original error: {e}", + ) + raise def _resolve_ids(self, target_ids: numpy.ndarray) -> numpy.ndarray: """Convert target IDs to row indices. @@ -257,34 +375,55 @@ def rank(self, variable_name: str, period) -> numpy.ndarray: class _ChainedGetter: """Intermediate object for link chaining: ``person.mother.household``.""" - def __init__(self, outer_link: Many2OneLink, inner_link: Link) -> None: + def __init__( + self, + outer_link: Many2OneLink, + inner_link: Link, + *, + chain: list[Link] | None = None, + ) -> None: self._outer = outer_link self._inner = inner_link + self._chain = chain if chain is not None else [outer_link, inner_link] - def get(self, variable_name: str, period) -> numpy.ndarray: + def get( + self, + variable_name: str, + period, + options=None, + ) -> numpy.ndarray: """Resolve ``person.mother.household.get("rent", period)``.""" - # 1. Resolve inner link value on inner entity - inner_values = self._inner.get(variable_name, period) - - # 2. Map back through outer link - target_ids = self._outer._source_population.simulation.calculate( - self._outer.link_field, - period, - ) - target_rows = self._outer._resolve_ids(target_ids) - - result = numpy.full( - self._outer._source_population.count, - 0, - dtype=inner_values.dtype, - ) - valid = target_rows >= 0 - result[valid] = inner_values[target_rows[valid]] + from openfisca_core.indexed_enums import EnumArray + + result = self._chain[-1].get(variable_name, period, options=options) + for link in reversed(self._chain[:-1]): + target_ids = link._source_population.simulation.calculate( + link.link_field, + period, + ) + target_rows = link._resolve_ids(target_ids) + mapped = numpy.full( + link._source_population.count, + 0, + dtype=result.dtype, + ) + valid = target_rows >= 0 + mapped[valid] = result[target_rows[valid]] + if isinstance(result, EnumArray): + mapped = EnumArray(mapped, result.possible_values) + result = mapped return result - def __call__(self, variable_name: str, period) -> numpy.ndarray: + def __call__( + self, + variable_name: str, + period, + *, + options=None, + **kwargs, + ) -> numpy.ndarray: """Shorthand for get(): ``person.mother.household("rent", period)``.""" - return self.get(variable_name, period) + return self.get(variable_name, period, options=options) def __getattr__(self, name: str): """Continue chaining: ``person.mother.household.region``.""" @@ -297,7 +436,11 @@ def __getattr__(self, name: str): if hasattr(target_pop, "links") and name in target_pop.links: next_link = target_pop.links[name] - return _ChainedGetter(self._outer, next_link) + return _ChainedGetter( + self._outer, + next_link, + chain=self._chain + [next_link], + ) target_entity = target_pop.entity raise AttributeError(f"Entity '{target_entity.key}' has no link named '{name}'") diff --git a/openfisca_core/links/tests/test_edge_cases.py b/openfisca_core/links/tests/test_edge_cases.py index 1fd97ee20..04346dfa8 100644 --- a/openfisca_core/links/tests/test_edge_cases.py +++ b/openfisca_core/links/tests/test_edge_cases.py @@ -154,6 +154,51 @@ def test_get_before_resolve_raises(self): with pytest.raises((AttributeError, TypeError)): link.get("salary", "2024") + def test_unresolved_link_missing_target_entity(self): + """resolve() should raise a clear KeyError when target entity is absent.""" + link = Many2OneLink("broken", "mother_id", "unknown_entity") + with pytest.raises(KeyError, match="unknown_entity"): + link.resolve({}) + + +class TestLinkFieldNotFound: + def test_many2one_unknown_link_field_raises_clear_error(self): + """Unknown link_field should include link name and field in the error.""" + broken_link = Many2OneLink("mother", "nonexistent_variable", "person") + _, sim = _make_tbs_and_sim(n_persons=2, person_links=[broken_link]) + sim.set_input("salary", "2024", [100.0, 200.0]) + + with pytest.raises(Exception) as exc_info: + sim.persons.links["mother"].get("salary", "2024") + + message = str(exc_info.value) + assert "mother" in message + assert "nonexistent_variable" in message + + +class TestIdToRownumPartial: + def test_id_to_rownum_valid_mapping(self): + """A non-identity id_to_rownum mapping should be applied correctly.""" + mother_link = Many2OneLink("mother", "mother_id", "person") + _, sim = _make_tbs_and_sim(n_persons=3, person_links=[mother_link]) + sim.set_input("salary", "2024", [10.0, 20.0, 30.0]) + sim.set_input("mother_id", "2024", [0, 1, 2]) + sim.persons._id_to_rownum = numpy.array([2, 0, 1], dtype=numpy.intp) + + result = sim.persons.links["mother"].get("salary", "2024") + numpy.testing.assert_array_equal(result, [30.0, 10.0, 20.0]) + + def test_id_to_rownum_out_of_bounds_uses_default(self): + """IDs outside id_to_rownum bounds should resolve to variable default.""" + mother_link = Many2OneLink("mother", "mother_id", "person") + _, sim = _make_tbs_and_sim(n_persons=3, person_links=[mother_link]) + sim.set_input("salary", "2024", [10.0, 20.0, 30.0]) + sim.set_input("mother_id", "2024", [0, 99, 2]) + sim.persons._id_to_rownum = numpy.array([2, 0, 1], dtype=numpy.intp) + + result = sim.persons.links["mother"].get("salary", "2024") + numpy.testing.assert_array_equal(result, [30.0, 0.0, 20.0]) + # ────────────────────────────────────────────────────────────── # 4. Condition all-False / all-True @@ -294,7 +339,7 @@ class is_female(variables.Variable): ) # Create and bind implicit link - link = ImplicitOne2ManyLink("persons", "household") + link = ImplicitOne2ManyLink("persons", "household", "person") pop = sim.populations["household"] link.attach(pop) link.resolve(sim.populations) diff --git a/openfisca_core/links/tests/test_implicit.py b/openfisca_core/links/tests/test_implicit.py index cdce13240..0a2b9fd3b 100644 --- a/openfisca_core/links/tests/test_implicit.py +++ b/openfisca_core/links/tests/test_implicit.py @@ -62,7 +62,7 @@ def test_implicit_many2one(sim): def test_implicit_one2many(sim): - link = ImplicitOne2ManyLink("persons", "household") + link = ImplicitOne2ManyLink("persons", "household", "person") link.attach(sim.populations["household"]) link.resolve(sim.populations) @@ -74,3 +74,393 @@ def test_implicit_one2many(sim): counts = link.count("2024") assert numpy.array_equal(counts, [2, 1, 1]) + + +def test_implicit_one2many_with_non_default_person_key(): + """Regression test: entity key != 'person' must not crash. + + openfisca-france uses 'individu' as the person entity key. + Before the fix, ImplicitOne2ManyLink hardcoded target_entity_key='person', + causing a KeyError during link resolution. + """ + individu = entities.SingleEntity("individu", "individus", "Un individu", "") + menage = entities.GroupEntity( + "menage", + "menages", + "Un ménage", + "", + roles=[{"key": "personne_de_reference"}], + ) + + tbs = taxbenefitsystems.TaxBenefitSystem([individu, menage]) + + class salaire(variables.Variable): + value_type = float + entity = individu + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(salaire) + + sim = SimulationBuilder().build_from_dict( + tbs, + { + "individus": { + "i0": {"salaire": {"2024": 1000.0}}, + "i1": {"salaire": {"2024": 500.0}}, + }, + "menages": { + "m0": {"personne_de_reference": ["i0", "i1"]}, + }, + }, + ) + + # The implicit O2M link should resolve with target_entity_key='individu' + link = ImplicitOne2ManyLink("individus", "menage", "individu") + link.attach(sim.populations["menage"]) + link.resolve(sim.populations) # Would raise KeyError before the fix + + salaires = link.sum("salaire", "2024") + assert numpy.array_equal(salaires, [1500.0]) + + +def test_implicit_m2o_role_projector_projected_to_persons(): + """person.group.projector('var', period) must return one value per person (projected). + + When using an implicit M2O link and a projector (e.g. .first_person or + .demandeur), the result is at group level. It must be projected back to + the source (person) level so each person gets their group's value. + Regression test for projection being skipped when the projector has no + 'projectable' attribute (Projector instances). + """ + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + class rent(variables.Variable): + value_type = float + entity = household + definition_period = periods.DateUnit.YEAR + + for var in [age, rent]: + tbs.add_variable(var) + + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": { + "p0": {"age": {"2024": 40}}, + "p1": {"age": {"2024": 37}}, + "p2": {"age": {"2024": 54}}, + "p3": {"age": {"2024": 20}}, + }, + "households": { + "h0": {"member": ["p0", "p1"], "rent": {"2024": 800.0}}, + "h1": {"member": ["p2", "p3"], "rent": {"2024": 500.0}}, + }, + }, + ) + + # person.household.first_person('age', period) = first member's age per household, + # projected to persons. p0,p1 in h0 (first p0=40) -> [40, 40]; p2,p3 in h1 (first p2=54) -> [54, 54] + age_first = sim.persons.household.first_person("age", "2024") + assert age_first.shape == (4,), "Result must be projected to person count" + assert numpy.array_equal(age_first, [40, 40, 54, 54]) + + +def test_implicit_m2o_project_entity_sized_same_as_old_logic(): + """Entity-sized result is projected: behaviour unchanged from old logic. + + When a projector (e.g. first_person) returns one value per entity, + _project_implicit must project it to source (person) so each person + gets their entity's value — same as target.project(result). + """ + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(age) + + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": { + "p0": {"age": {"2024": 40}}, + "p1": {"age": {"2024": 37}}, + "p2": {"age": {"2024": 54}}, + "p3": {"age": {"2024": 20}}, + }, + "households": { + "h0": {"member": ["p0", "p1"]}, + "h1": {"member": ["p2", "p3"]}, + }, + }, + ) + + link = ImplicitMany2OneLink("household") + link.attach(sim.persons) + link.resolve(sim.populations) + + target = link._target_population + # Entity-sized: 2 households -> 2 values + entity_result = numpy.array([100.0, 200.0]) + assert entity_result.size == target.count + + projected = link._project_implicit(entity_result) + # Old logic: target.project(result) -> each person gets their household's value + expected = target.project(entity_result) + assert numpy.array_equal(projected, expected) + assert numpy.array_equal(projected, [100.0, 100.0, 200.0, 200.0]) + + +def test_implicit_m2o_members_sized_returned_unchanged(): + """Members-sized result is returned as-is (regression for France maj_nbp). + + When a wrapped callable returns one value per member (e.g. like + famille.members('activite') in openfisca-france), the result is already + in source (person) space. Passing it to project() would raise + InvalidArraySizeError. We must return it unchanged. + """ + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(age) + + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": { + "p0": {"age": {"2024": 40}}, + "p1": {"age": {"2024": 37}}, + "p2": {"age": {"2024": 54}}, + "p3": {"age": {"2024": 20}}, + }, + "households": { + "h0": {"member": ["p0", "p1"]}, + "h1": {"member": ["p2", "p3"]}, + }, + }, + ) + + link = ImplicitMany2OneLink("household") + link.attach(sim.persons) + link.resolve(sim.populations) + + target = link._target_population + # Members-sized: 4 persons -> 4 values (e.g. activite per person) + members_result = numpy.array([10.0, 20.0, 30.0, 40.0]) + assert members_result.size == target.members.count + + out = link._project_implicit(members_result) + assert out is members_result or numpy.array_equal(out, members_result) + assert numpy.array_equal(out, [10.0, 20.0, 30.0, 40.0]) + + +def test_implicit_m2o_project_implicit_rejects_wrong_size(): + """_project_implicit raises ValueError when result size matches neither entity nor members.""" + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": {"p0": {}, "p1": {}}, + "households": {"h0": {"member": ["p0", "p1"]}}, + }, + ) + + link = ImplicitMany2OneLink("household") + link.attach(sim.persons) + link.resolve(sim.populations) + + with pytest.raises(ValueError, match="result size .* does not match"): + link._project_implicit(numpy.array([1.0, 2.0, 3.0])) # size 3, neither 1 nor 2 + + +def test_implicit_m2o_role_projector_has_has_role(): + """person.group.role_projector must expose has_role (e.g. for .demandeur.has_role(...)). + + When using an implicit M2O link, the wrapped projector (e.g. .demandeur) is + returned as _CallableProxy so it remains callable and also exposes attributes + from the original projector (e.g. has_role). Regression test for openfisca-france + patterns like individu.famille.demandeur.has_role(FoyerFiscal.DECLARANT_PRINCIPAL). + """ + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", + "households", + "A household", + "", + roles=[{"key": "demandeur", "max": 1}, {"key": "member"}], + ) + + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(age) + + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": { + "p0": {"age": {"2024": 40}}, + "p1": {"age": {"2024": 37}}, + "p2": {"age": {"2024": 54}}, + "p3": {"age": {"2024": 20}}, + }, + "households": { + "h0": {"demandeur": ["p0"], "member": ["p0", "p1"]}, + "h1": {"demandeur": ["p2"], "member": ["p2", "p3"]}, + }, + }, + ) + + # person.household.demandeur: must be callable (projector) and have has_role + demandeur_proxy = sim.persons.household.demandeur + assert callable(demandeur_proxy), "person.household.demandeur must be callable" + assert hasattr( + demandeur_proxy, "has_role" + ), "person.household.demandeur must have has_role" + + # has_role(role) must return a boolean array (one per person) + demandeur_role = household.DEMANDEUR + is_demandeur = demandeur_proxy.has_role(demandeur_role) + assert is_demandeur.shape == (4,), "has_role must return one value per person" + # p0 and p2 are demandeurs + assert numpy.array_equal(is_demandeur, [True, False, True, False]) + + # Callable behaviour unchanged: demandeur('age', period) returns ages projected to persons + age_demandeur = demandeur_proxy("age", "2024") + assert age_demandeur.shape == ( + 4, + ), "Call must return one value per person (projected)" + assert numpy.array_equal(age_demandeur, [40, 40, 54, 54]) + + +def test_implicit_m2o_sum_returns_person_sized(): + """person.group.sum(array, role=...) must return person-sized array (projected from entity). + + Regression test for openfisca-france: in an Individu formula, + individu.famille.sum(revenu_i, role=Famille.PARENT) must have shape (n_persons,) + so it can be combined with other person-sized terms (e.g. base_ressources). + + Old (buggy) behavior: accessing .sum on the link returned the target's sum directly, + so result was entity-sized (2,) and caused "operands could not be broadcast with shapes (5,) (2,)". + New (fixed) behavior: ImplicitMany2OneLink.sum projects the result to person size (5,). + """ + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", + "households", + "A household", + "", + roles=[{"key": "parent", "max": 2}, {"key": "child"}], + ) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": {"p0": {}, "p1": {}, "p2": {}, "p3": {}, "p4": {}}, + "households": { + "h0": {"parent": ["p0", "p1"], "child": []}, + "h1": {"parent": ["p2"], "child": ["p3", "p4"]}, + }, + }, + ) + # 5 persons, 2 households. Person-sized array (e.g. revenu per person) + revenu_i = numpy.array([100.0, 200.0, 300.0, 10.0, 20.0]) + assert revenu_i.shape == (5,) + + # Old impl: target.sum(...) returns entity-sized (2,) — would break when combined with (5,) in formulas + target_sum = sim.populations["household"].sum(revenu_i, role=household.PARENT) + assert target_sum.shape == ( + 2, + ), "raw target sum is entity-sized (old behavior if used from link)" + + # New impl: person.household.sum(..., role=parent) returns (5,) so each person gets their household's parent sum + sum_parent = sim.persons.household.sum(revenu_i, role=household.PARENT) + assert sum_parent.shape == ( + 5, + ), "sum must be projected to person size (5,) not entity size (2,)" + # h0 parents: p0=100, p1=200 -> sum 300. h1 parents: p2=300 -> sum 300. + assert numpy.array_equal(sum_parent, [300.0, 300.0, 300.0, 300.0, 300.0]) + + +def test_implicit_m2o_role_projector_chained_get_returns_person_sized(): + """person.group.demandeur.other_link.get(...) must return person-sized (projected). + + Regression test for openfisca-france: in an Individu formula, + individu.famille.demandeur.foyer_fiscal('aide_logement_base_revenus_fiscaux', period) + must have shape (n_persons,) so it can be combined with other person-sized terms. + + Old (buggy) behavior: the chained link's get() returned entity-sized (one per demandeur), + causing broadcast errors. New (fixed) behavior: _CallableProxy wraps link-like attributes + so get() results are projected via _project_implicit to person size. + """ + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", + "households", + "A household", + "", + roles=[{"key": "demandeur", "max": 1}, {"key": "member"}], + ) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(age) + + sim = SimulationBuilder().build_from_dict( + tbs, + { + "persons": { + "p0": {"age": {"2024": 40}}, + "p1": {"age": {"2024": 37}}, + "p2": {"age": {"2024": 54}}, + "p3": {"age": {"2024": 20}}, + }, + "households": { + "h0": {"demandeur": ["p0"], "member": ["p0", "p1"]}, + "h1": {"demandeur": ["p2"], "member": ["p2", "p3"]}, + }, + }, + ) + # person.household.demandeur('age', period) must return (4,) — callable result projected + demandeur_proxy = sim.persons.household.demandeur + age_demandeur = demandeur_proxy("age", "2024") + assert age_demandeur.shape == ( + 4, + ), "demandeur(...) must return person-sized (projected)" + assert numpy.array_equal(age_demandeur, [40, 40, 54, 54]) diff --git a/openfisca_core/links/tests/test_integration.py b/openfisca_core/links/tests/test_integration.py index c1dc160ff..b70892f5f 100644 --- a/openfisca_core/links/tests/test_integration.py +++ b/openfisca_core/links/tests/test_integration.py @@ -190,3 +190,99 @@ def test_no_links_no_problem(self): # Simulation should work fine assert sim.persons.count == 2 + + +# -- Regression test: non-default person entity key ----------------------- + + +class TestNonDefaultPersonKey: + """Verify that _resolve_links works when person entity key != 'person'. + + This is a regression test for the France API crash where the person + entity is named 'individu' instead of 'person'. + """ + + def test_resolve_links_with_individu_key(self): + """Simulation.__init__ must not crash with entity key 'individu'.""" + individu = entities.SingleEntity("individu", "individus", "Un individu", "") + menage = entities.GroupEntity( + "menage", + "menages", + "Un ménage", + "", + roles=[{"key": "personne_de_reference"}], + ) + tbs = taxbenefitsystems.TaxBenefitSystem([individu, menage]) + # This line triggered the KeyError before the fix + sim = SimulationBuilder().build_default_simulation(tbs, count=3) + + assert sim.persons.count == 3 + # Implicit links should be auto-generated with correct keys + assert "menage" in sim.persons.links # individu → menage (M2O) + assert ( + "individus" in sim.populations["menage"].links + ) # menage → individus (O2M) + + def test_implicit_link_target_uses_actual_person_key(self): + """The O2M link target must be 'individu', not 'person'.""" + individu = entities.SingleEntity("individu", "individus", "Un individu", "") + foyer_fiscal = entities.GroupEntity( + "foyer_fiscal", + "foyers_fiscaux", + "Un foyer fiscal", + "", + roles=[{"key": "declarant"}], + ) + tbs = taxbenefitsystems.TaxBenefitSystem([individu, foyer_fiscal]) + sim = SimulationBuilder().build_default_simulation(tbs, count=2) + + o2m_link = sim.populations["foyer_fiscal"].links["individus"] + assert o2m_link.target_entity_key == "individu" + assert o2m_link.is_resolved + assert o2m_link._target_population is sim.persons + + +def test_chained_link_three_levels(): + """person -> mother -> mother -> household should compose correctly.""" + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + + person.add_link(Many2OneLink("mother", "mother_id", "person")) + person.add_link(Many2OneLink("household", "household_id", "household")) + + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class mother_id(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.ETERNITY + default_value = -1 + + class household_id(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.ETERNITY + default_value = -1 + + class rent(variables.Variable): + value_type = float + entity = household + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(mother_id) + tbs.add_variable(household_id) + tbs.add_variable(rent) + + sim = SimulationBuilder().build_default_simulation( + tbs, + count=3, + group_members={"household": [0, 0, 0]}, + ) + sim.set_input("mother_id", "2024", [1, 2, -1]) + sim.set_input("household_id", "2024", [-1, -1, 0]) + sim.set_input("rent", "2024", [700.0]) + + result = sim.persons.links["mother"].mother.household.get("rent", "2024") + assert result[0] == pytest.approx(700.0) diff --git a/openfisca_core/links/tests/test_many2one.py b/openfisca_core/links/tests/test_many2one.py index 6bf102f4e..7eba43183 100644 --- a/openfisca_core/links/tests/test_many2one.py +++ b/openfisca_core/links/tests/test_many2one.py @@ -1,10 +1,16 @@ import numpy import pytest -from openfisca_core import entities, periods, taxbenefitsystems, variables +from openfisca_core import entities, periods, taxbenefitsystems, tools, variables +from openfisca_core.indexed_enums import Enum, EnumArray from openfisca_core.links import Many2OneLink +from openfisca_core.populations import ADD, DIVIDE +from openfisca_core.populations._errors import IncompatibleOptionsError from openfisca_core.simulations import SimulationBuilder +# Period used for options tests (mirrors test_countries.PERIOD) +PERIOD = periods.period("2024-01") + @pytest.fixture def sim(): @@ -54,18 +60,66 @@ class household_role(variables.Variable): for var in [age, rent, mother_id, household_id, household_role]: tbs.add_variable(var) - # persons: 0, 1, 2, 3 - # households: 0, 1 - sim = SimulationBuilder().build_default_simulation(tbs, count=4) + # persons: 0, 1, 2, 3 in 2 households (0,1 -> hh0; 2,3 -> hh1) + sim = SimulationBuilder().build_default_simulation( + tbs, count=4, group_members={"household": numpy.array([0, 0, 1, 1])} + ) # Mother of 0 is -1, 1 is 0, 2 is 0, 3 is 1 sim.set_input("mother_id", "2024", [-1, 0, 0, 1]) sim.set_input("age", "2024", [50, 25, 20, 5]) sim.set_input("household_id", "2024", [0, 0, 1, 1]) sim.set_input("household_role", "2024", [10, 20, 10, 20]) - sim.set_input("rent", "2024", [800.0, 500.0, 0.0, 0.0]) + # 2 households: rent per household (hh0=800, hh1=500) + sim.set_input("rent", "2024", [800.0, 500.0]) return sim +def test_many2one_get_uses_id_to_rownum(): + """Many2OneLink.get() uses target population _id_to_rownum when resolving IDs to rows. + + With a non-identity id_to_rownum (e.g. [2, 0, 1]), entity id 0 -> row 2, id 1 -> row 0, + id 2 -> row 1. So link.get() must index target values by id_to_rownum[target_id], + not by target_id directly. + """ + from openfisca_core import entities, periods, taxbenefitsystems, variables + from openfisca_core.links import Many2OneLink + from openfisca_core.simulations import SimulationBuilder + + person = entities.SingleEntity("person", "persons", "", "") + mother_link = Many2OneLink("mother", "mother_id", "person") + person.add_link(mother_link) + tbs = taxbenefitsystems.TaxBenefitSystem([person]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + class mother_id(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.ETERNITY + default_value = -1 + + tbs.add_variable(age) + tbs.add_variable(mother_id) + + sim = SimulationBuilder().build_default_simulation(tbs, count=3) + # Ages at rows 0,1,2 = 10, 20, 30 + sim.set_input("age", "2024", [10, 20, 30]) + # Person 0 -> mother id 0, person 1 -> mother id 1, person 2 -> mother id 2 + sim.set_input("mother_id", "2024", [0, 1, 2]) + + # Non-identity: entity id 0 -> row 2, id 1 -> row 0, id 2 -> row 1 + # So id 0 fetches row 2 (age 30), id 1 fetches row 0 (age 10), id 2 fetches row 1 (age 20) + sim.persons._id_to_rownum = numpy.array([2, 0, 1], dtype=numpy.intp) + + link = sim.persons.links["mother"] + mother_ages = link.get("age", "2024") + # Without id_to_rownum we would get [10, 20, 30]; with [2,0,1] we get row 2,0,1 = [30, 10, 20] + assert numpy.array_equal(mother_ages, [30.0, 10.0, 20.0]) + + def test_many2one_get_intra_entity(sim): """Test person -> person lookup (mother).""" link = sim.persons.links["mother"] @@ -95,6 +149,51 @@ def test_many2one_get_inter_entity(sim): assert numpy.array_equal(h_rents, [800.0, 800.0, 500.0, 500.0]) +def test_many2one_get_enum_variable_returns_enum_array(): + """Many2OneLink.get should preserve EnumArray when projecting target enums.""" + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + household_link = Many2OneLink("household", "household_id", "household") + person.add_link(household_link) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class occupancy_status(Enum): + tenant = "Tenant" + owner = "Owner" + + class household_id(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.ETERNITY + default_value = -1 + + class occupancy(variables.Variable): + value_type = Enum + possible_values = occupancy_status + default_value = occupancy_status.tenant + entity = household + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(household_id) + tbs.add_variable(occupancy) + + sim = SimulationBuilder().build_default_simulation( + tbs, + count=3, + group_members={"household": numpy.array([0, 1, 1])}, + ) + sim.set_input("household_id", "2024", [0, 1, 1]) + sim.set_input("occupancy", "2024", ["tenant", "owner"]) + + result = sim.persons.links["household"].get("occupancy", "2024") + assert isinstance(result, EnumArray) + numpy.testing.assert_array_equal( + result.decode_to_str(), ["tenant", "owner", "owner"] + ) + + def test_many2one_chaining(sim): """Test person.mother.household.rent chaining.""" mother_link = sim.persons.links["mother"] @@ -128,22 +227,144 @@ def test_many2one_role_helpers(sim): def test_many2one_rank(sim): """Ranking people by age within their household via the link. - The default ``sim`` fixture uses ``build_default_simulation`` which - does not populate ``household.members_entity_id`` correctly, so we - patch the group population manually using the input variable. + The fixture builds with ``group_members={"household": [0,0,1,1]}`` so + the group structure (4 persons → 2 households) is set at build time. """ - # ensure household group mappings match the input variable - sim.household.members_entity_id = sim.persons("household_id", "2024") - # reset any cached position so rank uses updated mapping - sim.household._members_position = None - link = sim.persons.links["household"] # ages [50, 25, 20, 5] per person ranks = link.rank("age", "2024") # households: h0->[50,25] -> ranks [1,0]; h1->[20,5] -> ranks [1,0] assert numpy.array_equal(ranks, [1, 0, 1, 0]) - # chaining should also forward to outer link (no behavioural change) + +# -- Many2OneLink options (ADD / DIVIDE) ----------------------------------- + + +def test_many2one_call_accepts_options_keyword(sim): + """Link __call__ accepts options= and forwards to get(); ADD over one year = same as calculate.""" + link = sim.persons.links["household"] + # rent is YEAR; ADD over "2024" (one subperiod) equals plain calculate + without_options = link("rent", "2024") + with_add = link("rent", "2024", options=[ADD]) + assert numpy.array_equal(without_options, with_add) + assert numpy.array_equal(link.get("rent", "2024", options=None), without_options) + + +def test_many2one_get_with_options_add(sim): + """Link.get(..., options=[ADD]) returns same as target population calculate_add projected.""" + link = sim.persons.links["household"] + # Direct get with options + result = link.get("rent", "2024", options=[ADD]) + # Should match household.calculate_add projected to persons via link + expected = sim.household("rent", "2024", options=[ADD]) + person_household_ids = sim.persons("household_id", "2024") + expected_per_person = numpy.array( + [expected[i] for i in person_household_ids], + dtype=expected.dtype, + ) + assert numpy.array_equal(result, expected_per_person) + + +def test_many2one_call_options_incompatible(sim): + """Link __call__ with both ADD and DIVIDE raises IncompatibleOptionsError.""" + link = sim.persons.links["household"] + with pytest.raises(IncompatibleOptionsError): + link("rent", "2024", options=[ADD, DIVIDE]) + + +def test_many2one_chained_call_with_options(sim): + """Chained link (person.mother.household) accepts options= in __call__ and get.""" chained = sim.persons.links["mother"].household - ranks2 = chained.rank("age", "2024") - assert numpy.array_equal(ranks2, ranks) + without_options = chained("rent", "2024") + with_add = chained("rent", "2024", options=[ADD]) + assert numpy.array_equal(without_options, with_add) + with_add_get = chained.get("rent", "2024", options=[ADD]) + assert numpy.array_equal(with_add_get, without_options) + + +def test_many2one_get_with_options_divide(sim): + """Link.get(..., options=[DIVIDE]) matches projector (mirrors test_calculate_divide).""" + link = sim.persons.links["household"] + # rent is YEAR; DIVIDE over a month gives rent/12 per person (same as test_countries) + result = link.get("rent", PERIOD, options=[DIVIDE]) + expected = sim.household("rent", PERIOD, options=[DIVIDE]) + person_household_ids = sim.persons("household_id", PERIOD) + expected_per_person = numpy.array( + [expected[i] for i in person_household_ids], + dtype=expected.dtype, + ) + tools.assert_near( + result, + expected_per_person, + absolute_error_margin=0.01, + ) + + +def test_many2one_divide_option_with_complex_period(sim): + """DIVIDE is only supported for a single subperiod; multi-unit period raises (mirrors test_divide_option_with_complex_period). + + This is intentional: DIVIDE gives e.g. yearly_value/12 for one month, not + for a quarter. Simulation.calculate_divide enforces period.size == 1 and + raises ValueError with a clear message. The link must propagate the same error. + """ + link = sim.persons.links["household"] + quarter = PERIOD.last_3_months # 3 months → period.size > 1 + + with pytest.raises(ValueError) as exc_info: + link("rent", quarter, options=[DIVIDE]) + + error_message = str(exc_info.value) + expected_words = ["Can't", "calculate", "month", "year"] + for word in expected_words: + assert ( + word in error_message + ), f"Expected '{word}' in error message '{error_message}'" + + +# -- User-facing errors for invalid use --------------------------------------- + + +def test_get_rank_requires_group_entity_raises(sim): + """get_rank(link to single entity) raises a clear ValueError.""" + # person.mother is a link to person (single entity); rank requires a group + chained = sim.persons.links["mother"].household + with pytest.raises(ValueError, match="get_rank requires a group entity"): + chained.rank("age", "2024") + + +def test_value_nth_person_inconsistent_group_raises(sim): + """value_nth_person raises clear ValueError when count != entities from members_entity_id.""" + # Create inconsistent state: members_entity_id implies 4 entities, count is 2 + sim.household.set_members_entity_id(numpy.array([0, 1, 2, 3])) + sim.household.count = 2 + link = sim.persons.links["household"] + with pytest.raises(ValueError, match="Group population .* is inconsistent"): + link.rank("age", "2024") + + +def test_callable_proxy_preserves_has_role(): + """_CallableProxy preserves attributes (e.g. has_role) from the wrapped projector. + + Old impl: link returned a bare function, so .demandeur.has_role(...) raised + AttributeError. New impl: link returns _CallableProxy(callable, target_attr) + so the result is callable and proxies has_role (and other attrs) to the + wrapped projector. + """ + from openfisca_core.links.many2one import _CallableProxy + + def call_(*args, **kwargs): + return numpy.array([1.0, 2.0, 3.0]) + + class MockProjector: + def has_role(self, role): + return numpy.array([True, False, True]) + + wrapped = MockProjector() + proxy = _CallableProxy(call_, wrapped) + + assert callable(proxy), "Proxy must be callable" + assert hasattr(proxy, "has_role"), "Proxy must expose has_role" + out = proxy("age", "2024") + assert numpy.array_equal(out, [1.0, 2.0, 3.0]) + is_role = proxy.has_role(None) + assert numpy.array_equal(is_role, [True, False, True]) diff --git a/openfisca_core/links/tests/test_one2many.py b/openfisca_core/links/tests/test_one2many.py index 973ddd8a8..cd351ad9f 100644 --- a/openfisca_core/links/tests/test_one2many.py +++ b/openfisca_core/links/tests/test_one2many.py @@ -1,7 +1,7 @@ import numpy import pytest -from openfisca_core import entities, periods, taxbenefitsystems, variables +from openfisca_core import entities, periods, taxbenefitsystems, tools, variables from openfisca_core.links import One2ManyLink from openfisca_core.simulations import SimulationBuilder @@ -60,16 +60,17 @@ class household_role(variables.Variable): # 0: parent, 1: child def test_one2many_aggregations(sim): + """Link sum/count/avg/min/max match projector semantics (mirrors simulation aggregation).""" link = sim.populations["household"].links["members"] res_sum = link.sum("salary", "2024") - assert numpy.array_equal(res_sum, [1500.0, 2000.0, 0.0, 0.0]) + tools.assert_near(res_sum, [1500.0, 2000.0, 0.0, 0.0], absolute_error_margin=0.01) res_count = link.count("2024") assert numpy.array_equal(res_count, [2, 1, 0, 0]) res_avg = link.avg("salary", "2024") - assert numpy.array_equal(res_avg, [750.0, 2000.0, 0.0, 0.0]) + tools.assert_near(res_avg, [750.0, 2000.0, 0.0, 0.0], absolute_error_margin=0.01) res_min = link.min("salary", "2024") assert numpy.array_equal(res_min, [500.0, 2000.0, 0.0, 0.0]) @@ -104,9 +105,10 @@ def test_one2many_role_filter(sim): def test_one2many_condition_filter(sim): + """Link sum with condition matches filtered aggregation (same as projector path).""" link = sim.populations["household"].links["members"] condition = sim.calculate("is_female", "2024") # Sum of salary for females only res_sum = link.sum("salary", "2024", condition=condition) - assert numpy.array_equal(res_sum, [1000.0, 0.0, 0.0, 0.0]) + tools.assert_near(res_sum, [1000.0, 0.0, 0.0, 0.0], absolute_error_margin=0.01) diff --git a/openfisca_core/populations/group_population.py b/openfisca_core/populations/group_population.py index a3be9539a..9e9542477 100644 --- a/openfisca_core/populations/group_population.py +++ b/openfisca_core/populations/group_population.py @@ -67,6 +67,29 @@ def members_entity_id(self): def members_entity_id(self, members_entity_id) -> None: self._members_entity_id = members_entity_id + def set_members_entity_id(self, members_entity_id) -> None: + """Set group structure from a person-indexed array of group entity ids. + + Updates ``members_entity_id`` and ``count`` (number of distinct groups), + and clears internal caches so rank, value_nth_person, etc. use the new + structure. Use this instead of assigning to ``members_entity_id`` and + clearing private cache attributes manually. + + Args: + members_entity_id: 1D array of length ``members.count``; value at + index i is the group entity id for person i. Group ids must be + 0-based contiguous (0 .. K-1 for K groups). + """ + arr = numpy.asarray(members_entity_id, dtype=numpy.int32) + if len(arr) == 0: + raise ValueError( + "members_entity_id cannot be empty — at least one member is required." + ) + self._members_entity_id = arr + self.count = int(numpy.max(arr)) + 1 + self._members_position = None + self._ordered_members_map = None + @property def members_role(self): if self._members_role is None: @@ -352,6 +375,14 @@ def value_nth_person(self, n, array, default=0): positions = self.members_position nb_persons_per_entity = self.nb_persons() members_map = self.ordered_members_map + nb_entities = len(nb_persons_per_entity) + if nb_entities != self.count: + raise ValueError( + f"Group population '{self.entity.key}' is inconsistent: " + f"count is {self.count} but members_entity_id implies " + f"{nb_entities} entities (from bincount). " + "Ensure count matches the number of entities implied by members_entity_id." + ) result = self.filled_array(default, dtype=array.dtype) # For households that have at least n persons, set the result as the value of criteria for the person for which the position is n. # The map is needed b/c the order of the nth persons of each household in the persons vector is not necessarily the same than the household order. diff --git a/openfisca_core/populations/population.py b/openfisca_core/populations/population.py index 9347fbd68..a119b26b2 100644 --- a/openfisca_core/populations/population.py +++ b/openfisca_core/populations/population.py @@ -119,7 +119,16 @@ def get_rank( elif hasattr(entity, "_target_population"): # Handle new Link system entity = entity._target_population - positions = entity.members_position + try: + positions = entity.members_position + except AttributeError: + entity_key = getattr(entity, "entity", entity) + key = getattr(entity_key, "key", str(entity_key)) + raise ValueError( + f"get_rank requires a group entity (with members). " + f"'{key}' is a single entity and has no members_position. " + "Use a link to a group entity (e.g. person.household), not to another person (e.g. person.mother)." + ) from None biggest_entity_size = numpy.max(positions) + 1 filtered_criteria = numpy.where(condition, criteria, numpy.inf) ids = entity.members_entity_id diff --git a/openfisca_core/populations/tests/test_members_position.py b/openfisca_core/populations/tests/test_members_position.py index dc703b450..d287ef7d8 100644 --- a/openfisca_core/populations/tests/test_members_position.py +++ b/openfisca_core/populations/tests/test_members_position.py @@ -1,6 +1,10 @@ """Tests for GroupPopulation.members_position vectorized computation.""" import numpy +import pytest + +from openfisca_core import entities, periods, taxbenefitsystems, variables +from openfisca_core.simulations import SimulationBuilder class TestMembersPosition: @@ -114,3 +118,47 @@ def test_dtype_is_int32(self): result = pop.members_position assert result.dtype == numpy.int32 + + def test_set_members_entity_id_empty_raises(self): + """set_members_entity_id([]) should raise a clear ValueError.""" + from openfisca_core.populations.group_population import GroupPopulation + + pop = GroupPopulation.__new__(GroupPopulation) + with pytest.raises(ValueError, match="cannot be empty"): + pop.set_members_entity_id([]) + + def test_set_members_entity_id_non_contiguous_count(self): + """Non-contiguous ids should be accepted; count is max(id)+1.""" + person = entities.SingleEntity("person", "persons", "A person", "") + household = entities.GroupEntity( + "household", "households", "A household", "", roles=[{"key": "member"}] + ) + tbs = taxbenefitsystems.TaxBenefitSystem([person, household]) + + class age(variables.Variable): + value_type = int + entity = person + definition_period = periods.DateUnit.YEAR + + tbs.add_variable(age) + + sim = SimulationBuilder().build_default_simulation( + tbs, + count=4, + group_members={"household": numpy.array([0, 0, 2, 2])}, + ) + sim.set_input("age", "2024", [10, 20, 30, 40]) + + assert sim.household.count == 3 # max([0,0,2,2]) + 1 + + first = sim.household.value_nth_person( + 0, sim.persons("age", "2024"), default=-1 + ) + second = sim.household.value_nth_person( + 1, sim.persons("age", "2024"), default=-1 + ) + numpy.testing.assert_array_equal(first, [10, -1, 30]) + numpy.testing.assert_array_equal(second, [20, -1, 40]) + + ranks = sim.persons.get_rank(sim.household, sim.persons("age", "2024")) + numpy.testing.assert_array_equal(ranks, [0, 1, 0, 1]) diff --git a/openfisca_core/simulations/_build_default_simulation.py b/openfisca_core/simulations/_build_default_simulation.py index ac03ba1b5..75200a77f 100644 --- a/openfisca_core/simulations/_build_default_simulation.py +++ b/openfisca_core/simulations/_build_default_simulation.py @@ -1,5 +1,7 @@ """This module contains the _BuildDefaultSimulation class.""" +from __future__ import annotations + from typing import Union from typing_extensions import Self @@ -14,7 +16,12 @@ class _BuildDefaultSimulation: Args: tax_benefit_system(TaxBenefitSystem): The tax-benefit system. - count(int): The number of periods. + count(int): The number of persons (and, by default, of each group entity). + group_members: Optional mapping from group entity key to a 1D array of + group entity id per person (length ``count``). When provided, that + group's ``members_entity_id`` and ``count`` are set from the array + instead of the default (one person per group). Use this when tests + or examples need a specific grouping (e.g. 4 persons in 2 households). Examples: >>> from openfisca_core import entities, taxbenefitsystems @@ -47,14 +54,23 @@ class _BuildDefaultSimulation: #: The number of Population. count: int + #: Optional per-group entity key -> array of group id per person. + group_members: dict[str, numpy.ndarray] | None + #: The built populations. populations: dict[str, Union[Population[Entity]]] #: The built simulation. simulation: Simulation - def __init__(self, tax_benefit_system: TaxBenefitSystem, count: int) -> None: + def __init__( + self, + tax_benefit_system: TaxBenefitSystem, + count: int, + group_members: dict[str, numpy.ndarray] | None = None, + ) -> None: self.count = count + self.group_members = group_members self.populations = tax_benefit_system.instantiate_entities() self.simulation = Simulation(tax_benefit_system, self.populations) @@ -133,9 +149,11 @@ def add_id_to_rownum(self) -> Self: return self def add_members_entity_id(self) -> Self: - """Add ??? + """Set group populations' members_entity_id (and count when using group_members). - Each SingleEntity has its own GroupEntity. + Default: each person in their own group (members_entity_id = 0..count-1). + When ``group_members`` was passed to the builder, each listed group uses + the given array and its count is derived from it (clearing internal caches). Returns: _BuildDefaultSimulation: The builder. @@ -164,7 +182,19 @@ def add_members_entity_id(self) -> Self: """ for population in self.populations.values(): - if hasattr(population, "members_entity_id"): + if not hasattr(population, "members_entity_id"): + continue + key = population.entity.key + if self.group_members and key in self.group_members: + arr = numpy.asarray(self.group_members[key], dtype=numpy.int32) + if hasattr(population, "set_members_entity_id"): + population.set_members_entity_id(arr) + else: + population.members_entity_id = arr + population.count = int(numpy.max(arr)) + 1 + population._members_position = None + population._ordered_members_map = None + else: population.members_entity_id = numpy.array(range(self.count)) return self diff --git a/openfisca_core/simulations/simulation.py b/openfisca_core/simulations/simulation.py index be5948919..93f2eafa8 100644 --- a/openfisca_core/simulations/simulation.py +++ b/openfisca_core/simulations/simulation.py @@ -92,31 +92,47 @@ def _resolve_links(self) -> None: This is called once at ``__init__`` time, after ``link_to_entities_instances`` and ``create_shortcuts``. + + Backward-compatible: if an entity has no ``links`` or ``get_link`` + (e.g. older core or country package), link resolution is skipped for + that entity so CI and minimal-dependency setups keep working. """ - from openfisca_core.links.implicit import ( - ImplicitMany2OneLink, - ImplicitOne2ManyLink, - ) from openfisca_core.populations.group_population import GroupPopulation person_entity = self.persons.entity + has_links_api = ( + getattr(person_entity, "get_link", None) is not None + and getattr(person_entity, "add_link", None) is not None + ) - # Auto-generate implicit links - for population in self.populations.values(): - if not isinstance(population, GroupPopulation): - continue - group_key = population.entity.key - - # person -> group (Many2One) - if not person_entity.get_link(group_key): - m2o = ImplicitMany2OneLink(group_key) - person_entity.add_link(m2o) + if has_links_api: + from openfisca_core.links.implicit import ( + ImplicitMany2OneLink, + ImplicitOne2ManyLink, + ) - # group -> persons (One2Many) - o2m_name = person_entity.plural - if not population.entity.get_link(o2m_name): - o2m = ImplicitOne2ManyLink(o2m_name, group_key) - population.entity.add_link(o2m) + # Auto-generate implicit links + for population in self.populations.values(): + if not isinstance(population, GroupPopulation): + continue + group_key = population.entity.key + group_entity = population.entity + if ( + getattr(group_entity, "get_link", None) is None + or getattr(group_entity, "add_link", None) is None + ): + continue + + # person -> group (Many2One) + if not person_entity.get_link(group_key): + m2o = ImplicitMany2OneLink(group_key) + person_entity.add_link(m2o) + + # group -> persons (One2Many) + o2m_name = person_entity.plural + if not group_entity.get_link(o2m_name): + o2m = ImplicitOne2ManyLink(o2m_name, group_key, person_entity.key) + group_entity.add_link(o2m) from copy import copy @@ -124,7 +140,10 @@ def _resolve_links(self) -> None: for _key, population in self.populations.items(): entity = population.entity population.links = {} - for name, link in entity.links.items(): + entity_links = getattr(entity, "links", None) + if entity_links is None: + continue + for name, link in entity_links.items(): bound_link = copy(link) bound_link.attach(population) bound_link.resolve(self.populations) diff --git a/openfisca_core/simulations/simulation_builder.py b/openfisca_core/simulations/simulation_builder.py index 636ad7fd4..5efd90b97 100644 --- a/openfisca_core/simulations/simulation_builder.py +++ b/openfisca_core/simulations/simulation_builder.py @@ -1,6 +1,6 @@ from __future__ import annotations -from collections.abc import Iterable, Sequence +from collections.abc import Iterable, Mapping, Sequence from numpy.typing import NDArray as Array from typing import NoReturn @@ -147,12 +147,22 @@ def build_from_dict( params = self.explicit_singular_entities(tax_benefit_system, input_dict) return self.build_from_entities(tax_benefit_system, params) - if are_entities_fully_specified(params := input_dict, plural): - return self.build_from_entities(tax_benefit_system, params) + if are_entities_fully_specified(input_dict, plural): + return self.build_from_entities(tax_benefit_system, input_dict) + + if not are_entities_specified(input_dict, variables): + return self.build_from_variables(tax_benefit_system, input_dict) + + from openfisca_core.errors import SituationParsingError - if not are_entities_specified(params := input_dict, variables): - return self.build_from_variables(tax_benefit_system, params) - return None + raise SituationParsingError( + ["input"], + "Test input does not match any known format. " + f"Input keys: {list(input_dict)}. " + f"This may mean the wrong country package is in use: expected entity keys " + f"singular in {set(singular)} or plural in {set(plural)}. " + "Use the -c / --country-package option if you have several OpenFisca packages installed.", + ) def build_from_entities( self, @@ -313,17 +323,28 @@ def build_from_variables( def build_default_simulation( tax_benefit_system: TaxBenefitSystem, count: int = 1, + group_members: Mapping[str, numpy.ndarray] | None = None, ) -> Simulation: """Build a default simulation. Where: - - There are ``count`` persons - - There are ``count`` of each group entity, containing one person - - Every person has, in each entity, the first role + - There are ``count`` persons. + - By default, there are ``count`` of each group entity (one person per + group). Pass ``group_members`` to use a different structure, e.g. + ``{"household": numpy.array([0, 0, 1, 1])}`` for 4 persons in 2 + households. + - Every person has, in each entity, the first role. + + Args: + tax_benefit_system: The tax and benefit system. + count: Number of persons. + group_members: Optional mapping from group entity key to a 1D array + of group entity id per person (length ``count``). Group ids must + be 0-based contiguous. """ return ( - _BuildDefaultSimulation(tax_benefit_system, count) + _BuildDefaultSimulation(tax_benefit_system, count, group_members) .add_count() .add_ids() .add_members_entity_id() diff --git a/pyproject.toml b/pyproject.toml index b2b82088e..f46ff1b76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,6 +4,7 @@ target-version = [ "py310", "py311", "py312", "py313" ] [tool.codespell] ignore-words-list = [ "THIRDPARTY", + "activite", "ans", "constitue", "exemple", diff --git a/tests/core/test_asof_variable.py b/tests/core/test_asof_variable.py index 875695b22..c0c4b6da5 100644 --- a/tests/core/test_asof_variable.py +++ b/tests/core/test_asof_variable.py @@ -17,6 +17,8 @@ from openfisca_core.holders import Holder from openfisca_core.periods import DateUnit, period from openfisca_core.populations import Population +from openfisca_core.simulations import Simulation +from openfisca_core.taxbenefitsystems import TaxBenefitSystem from openfisca_core.variables import Variable # --------------------------------------------------------------------------- @@ -81,6 +83,19 @@ class _StubSimulation: return Holder(var, population) +def _make_simulation(*variable_classes, count: int = 3) -> Simulation: + """Build a minimal Simulation with the given variable classes.""" + tbs = TaxBenefitSystem([_entity]) + person_entity = tbs.person_entity + for vc in variable_classes: + tbs.add_variable(vc) + pop = Population(person_entity) + pop.count = count + pop.ids = [str(i) for i in range(count)] + sim = Simulation(tbs, {person_entity.key: pop}) + return sim + + # --------------------------------------------------------------------------- # 1. Value persists forward in time # --------------------------------------------------------------------------- @@ -561,3 +576,127 @@ class _MultiSnapVariable(Variable): holder.get_array(period("2023-03")), [0, 0] # before any patch → base ) numpy.testing.assert_array_equal(holder.get_array(period("2024-02")), [2, 20]) + + +def test_asof_snapshot_fifo_eviction_order(): + """Only the two most recently inserted snapshots should remain in cache.""" + + class _TinySnapshotVariable(Variable): + entity = _entity + definition_period = DateUnit.MONTH + value_type = int + as_of = "start" + snapshot_count = 2 + + holder = _make_holder(_TinySnapshotVariable, count=2) + holder.set_input("2024-01", numpy.array([1, 10])) + holder.set_input("2024-02", numpy.array([2, 20])) + holder.set_input("2024-03", numpy.array([3, 30])) + holder.set_input("2024-04", numpy.array([4, 40])) + holder.set_input("2024-05", numpy.array([5, 50])) + + cached_instants = list(holder._as_of_snapshots.keys()) + expected = [period("2024-04").start, period("2024-05").start] + assert cached_instants == expected + numpy.testing.assert_array_equal(holder.get_array(period("2024-01")), [1, 10]) + + +def test_asof_snapshot_eviction_is_fifo_not_lru(): + """Reading an old snapshot must not protect it from eviction.""" + + class _TinySnapshotVariable(Variable): + entity = _entity + definition_period = DateUnit.MONTH + value_type = int + as_of = "start" + snapshot_count = 3 + + holder = _make_holder(_TinySnapshotVariable, count=2) + holder.set_input("2024-01", numpy.array([1, 10])) + holder.set_input("2024-02", numpy.array([2, 20])) + holder.set_input("2024-03", numpy.array([3, 30])) + assert list(holder._as_of_snapshots.keys()) == [ + period("2024-01").start, + period("2024-02").start, + period("2024-03").start, + ] + + # Touch the oldest snapshot while it is still cached. + numpy.testing.assert_array_equal(holder.get_array(period("2024-01")), [1, 10]) + holder.set_input("2024-04", numpy.array([4, 40])) + + cached_instants = list(holder._as_of_snapshots.keys()) + assert period("2024-01").start not in cached_instants + assert period("2024-02").start in cached_instants + + +def test_asof_retroactive_patch_evicts_later_snapshots(): + """A retroactive patch should evict later snapshots that became stale.""" + holder = _make_holder(_AsOfIntVariable, count=2) + holder.set_input("2024-01", numpy.array([10, 20])) # base + holder.set_input("2024-03", numpy.array([10, 30])) # patch at Mar + numpy.testing.assert_array_equal(holder.get_array(period("2024-03")), [10, 30]) + assert period("2024-03").start in holder._as_of_snapshots + + holder.set_input("2024-02", numpy.array([99, 20])) # retroactive patch + assert period("2024-03").start not in holder._as_of_snapshots + + numpy.testing.assert_array_equal(holder.get_array(period("2024-03")), [99, 30]) + + +def test_asof_retroactive_patch_value_correctness(): + """Values should reflect base + retroactive patch + later patch in order.""" + holder = _make_holder(_AsOfIntVariable, count=2) + holder.set_input("2024-01", numpy.array([10, 20])) # base + holder.set_input("2024-03", numpy.array([10, 30])) # mar patch + holder.set_input("2024-02", numpy.array([99, 20])) # feb retro patch + + numpy.testing.assert_array_equal(holder.get_array(period("2024-01")), [10, 20]) + numpy.testing.assert_array_equal(holder.get_array(period("2024-02")), [99, 20]) + numpy.testing.assert_array_equal(holder.get_array(period("2024-03")), [99, 30]) + + +def test_asof_backward_access_after_forward_is_correct(): + """Backward jump after forward read should still return the base state.""" + holder = _make_holder(_AsOfIntVariable, count=2) + holder.set_input("2024-01", numpy.array([10, 20])) + holder.set_input("2024-03", numpy.array([99, 20])) + + numpy.testing.assert_array_equal(holder.get_array(period("2024-03")), [99, 20]) + numpy.testing.assert_array_equal(holder.get_array(period("2024-01")), [10, 20]) + + +def test_set_input_sparse_without_base_raises_clear_error(): + """set_input_sparse before base should mention set_input requirement.""" + holder = _make_holder(_AsOfIntVariable) + with pytest.raises(ValueError, match="set_input first"): + holder.set_input_sparse("2024-01", numpy.array([0]), numpy.array([42])) + + +def test_initial_formula_runs_before_transition_formula(): + """initial_formula seeds P1; transition_formula is then used on P2/P3.""" + call_counts = {"initial": 0, "transition": 0} + + class Score(Variable): + entity = _entity + definition_period = DateUnit.MONTH + value_type = int + as_of = "start" + + def initial_formula(person, period): # noqa: N805 + call_counts["initial"] += 1 + return numpy.array([100, 100, 100]) + + def transition_formula(person, period): # noqa: N805 + call_counts["transition"] += 1 + previous = person("Score", period.last_month) + idx = numpy.arange(len(previous)) + return idx, previous + 1 + + sim = _make_simulation(Score) + numpy.testing.assert_array_equal(sim.calculate("Score", "2024-01"), [100, 100, 100]) + numpy.testing.assert_array_equal(sim.calculate("Score", "2024-02"), [101, 101, 101]) + numpy.testing.assert_array_equal(sim.calculate("Score", "2024-03"), [102, 102, 102]) + + assert call_counts["initial"] == 1 + assert call_counts["transition"] == 2 diff --git a/tests/core/test_link_accessors.py b/tests/core/test_link_accessors.py index 3d4570524..8ae6fef45 100644 --- a/tests/core/test_link_accessors.py +++ b/tests/core/test_link_accessors.py @@ -53,7 +53,7 @@ class rent(variables.Variable): def test_nth_accessor(simple_sim): - link = ImplicitOne2ManyLink("persons", "household") + link = ImplicitOne2ManyLink("persons", "household", "person") link.attach(simple_sim.populations["household"]) link.resolve(simple_sim.populations) @@ -67,7 +67,7 @@ def test_nth_accessor(simple_sim): def test_one2many_get_by_role(simple_sim): - link = ImplicitOne2ManyLink("persons", "household") + link = ImplicitOne2ManyLink("persons", "household", "person") link.attach(simple_sim.populations["household"]) link.resolve(simple_sim.populations)