BREAKING CHANGE: remove CoreEntity.check_variable_defined_for_entity

The feature is still provided by `CoreEntity.get_variable` when passed
the optional positional or keyword argument `check_existence`. In fact,
the removed method was duplicating and already existing behaviour.

Author: bonjourmauko

openfisca_core/entities/_core_entity.py

@@ -4,7 +4,6 @@
 from typing import ClassVar
 
 import abc
-import os
 
 from . import types as t
 from ._errors import TaxBenefitSystemUnsetError, VariableNotFoundError
@@ -103,6 +102,9 @@ def variables(self, /) -> Mapping[t.VariableName, t.Variable]:
             >>> that.variables
             {'tax': <openfisca_core.entities._core_entity.tax object at ...>}
 
+            >>> that.variables["tax"]
+            <openfisca_core.entities._core_entity.tax object at ...>
+
         """
         if self.tax_benefit_system is None:
             raise TaxBenefitSystemUnsetError
@@ -115,6 +117,7 @@ def variables(self, /) -> Mapping[t.VariableName, t.Variable]:
     def get_variable(
         self,
         variable_name: t.VariableName,
+        /,
         check_existence: bool = False,
     ) -> None | t.Variable:
         """Get ``variable_name`` from ``variables``.
@@ -173,81 +176,11 @@ def get_variable(
         """
         if self.tax_benefit_system is None:
             raise TaxBenefitSystemUnsetError
-        if not check_existence:
-            return self.variables.get(variable_name)
-        try:
-            return self.variables[variable_name]
-        except KeyError as error:
-            raise VariableNotFoundError(variable_name, self.plural) from error
-
-    def check_variable_defined_for_entity(self, variable_name: t.VariableName) -> None:
-        """Check if ``variable_name`` is defined for ``self``.
-
-        Args:
-            variable_name: The ``Variable`` to be found.
-
-        Raises:
-            ValueError: When the ``Variable`` exists but is defined
-                for another ``Entity``.
-
-        Examples:
-            >>> from openfisca_core import (
-            ...     entities,
-            ...     periods,
-            ...     taxbenefitsystems,
-            ...     variables,
-            ... )
-
-            >>> this = entities.SingleEntity("this", "", "", "")
-            >>> that = entities.SingleEntity("that", "", "", "")
-            >>> tax_benefit_system = taxbenefitsystems.TaxBenefitSystem([that])
-            >>> this.tax_benefit_system = tax_benefit_system
-
-            >>> this.check_variable_defined_for_entity("tax")
-            Traceback (most recent call last):
-            VariableNotFoundError: You tried to calculate or to set a value...
-
-            >>> class tax(variables.Variable):
-            ...     definition_period = periods.WEEK
-            ...     value_type = int
-            ...     entity = that
-
-            >>> this.tax_benefit_system.add_variable(tax)
-            <openfisca_core.entities._core_entity.tax object at ...>
-
-            >>> this.check_variable_defined_for_entity("tax")
-            Traceback (most recent call last):
-            ValueError: You tried to compute the variable 'tax' for the enti...
-
-            >>> tax.entity = this
-
-            >>> this.tax_benefit_system.update_variable(tax)
-            <openfisca_core.entities._core_entity.tax object at ...>
-
-            >>> this.check_variable_defined_for_entity("tax")
-
-        """
-        entity: None | t.CoreEntity = None
-        variable: None | t.Variable = self.get_variable(
-            variable_name,
-            check_existence=True,
-        )
-
-        if variable is not None:
-            entity = variable.entity
-
-        if entity is None:
-            return
-
-        if entity.key != self.key:
-            message = (
-                f"You tried to compute the variable '{variable_name}' for",
-                f"the entity '{self.plural}'; however the variable",
-                f"'{variable_name}' is defined for '{entity.plural}'.",
-                "Learn more about entities in our documentation:",
-                "<https://openfisca.org/doc/coding-the-legislation/50_entities.html>.",
-            )
-            raise ValueError(os.linesep.join(message))
+        if (variable := self.variables.get(variable_name)) is not None:
+            return variable
+        if check_existence:
+            raise VariableNotFoundError(variable_name, self.plural)
+        return None
 
 
 __all__ = ["CoreEntity"]

openfisca_core/entities/_errors.py

@@ -12,7 +12,7 @@ def __init__(self) -> None:
         super().__init__(msg)
 
 
-class VariableNotFoundError(IndexError):
+class VariableNotFoundError(ValueError):
     """Raised when a requested variable is not defined for as entity."""
 
     def __init__(self, name: t.VariableName, plural: t.EntityPlural) -> None:
@@ -23,8 +23,8 @@ def __init__(self, name: t.VariableName, plural: t.EntityPlural) -> None:
             "this is most probably linked to an update of the tax and "
             "benefit system. Look at its CHANGELOG to learn about renames "
             "and removals and update your code accordingly."
-            "Learn more about entities in our documentation:",
-            "<https://openfisca.org/doc/coding-the-legislation/50_entities.html>.",
+            "Learn more about entities in our documentation:"
+            "<https://openfisca.org/doc/coding-the-legislation/50_entities.html>."
         )
         super().__init__(msg)
 

openfisca_core/populations/_core_population.py

@@ -142,7 +142,7 @@ def __call__(
             option=options,
         )
 
-        self.entity.check_variable_defined_for_entity(calculate.variable)
+        self.entity.get_variable(calculate.variable, check_existence=True)
         self.check_period_validity(calculate.variable, calculate.period)
 
         if not isinstance(calculate.option, Sequence):
@@ -380,7 +380,7 @@ def get_holder(self, variable_name: t.VariableName) -> t.Holder:
             <openfisca_core.holders.holder.Holder object at ...
 
         """
-        self.entity.check_variable_defined_for_entity(variable_name)
+        self.entity.get_variable(variable_name, check_existence=True)
         holder = self._holders.get(variable_name)
         if holder:
             return holder

openfisca_core/simulations/simulation_builder.py

@@ -592,7 +592,7 @@ def init_variable_values(self, entity, instance_object, instance_id) -> None:
         for variable_name, variable_values in instance_object.items():
             path_in_json = [entity.plural, instance_id, variable_name]
             try:
-                entity.check_variable_defined_for_entity(variable_name)
+                entity.get_variable(variable_name, check_existence=True)
             except ValueError as e:  # The variable is defined for another entity
                 raise errors.SituationParsingError(path_in_json, e.args[0])
             except errors.VariableNotFoundError as e:  # The variable doesn't exist
@@ -665,6 +665,8 @@ def finalize_variables_init(self, population) -> None:
         for variable_name in self.input_buffer:
             try:
                 holder = population.get_holder(variable_name)
+            # TODO(Mauko Quiroga-Alvarado): We should handle this differently.
+            # It cost me a lot of time to figure out that this was the problem.
             except ValueError:  # Wrong entity, we can just ignore that
                 continue
             buffer = self.input_buffer[variable_name]

openfisca_core/types.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from collections.abc import Iterable, Sequence, Sized
+from collections.abc import Iterable, Mapping, Sequence, Sized
 from numpy.typing import DTypeLike, NDArray
 from typing import NewType, TypeVar, Union
 from typing_extensions import Protocol, Required, Self, TypeAlias, TypedDict
@@ -71,19 +71,23 @@
 
 
 class CoreEntity(Protocol):
-    key: EntityKey
-    plural: EntityPlural
-
-    def check_variable_defined_for_entity(
-        self,
-        variable_name: VariableName,
-        /,
-    ) -> None: ...
+    @property
+    def key(self, /) -> EntityKey: ...
+    @property
+    def plural(self, /) -> EntityPlural: ...
+    @property
+    def label(self, /) -> str: ...
+    @property
+    def doc(self, /) -> str: ...
+    @property
+    def tax_benefit_system(self, /) -> None | TaxBenefitSystem: ...
+    @property
+    def variables(self, /) -> Mapping[VariableName, Variable]: ...
     def get_variable(
         self,
-        variable_name: VariableName,
-        check_existence: bool = ...,
+        name: VariableName,
         /,
+        check_existence: bool = ...,
     ) -> None | Variable: ...
 
 

tests/fixtures/entities.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import pytest
 
 from openfisca_core.entities import Entity, GroupEntity
@@ -10,28 +12,22 @@ def get_variable(
         self,
         variable_name: str,
         check_existence: bool = False,
-    ) -> TestVariable:
+    ) -> None | TestVariable:
         result = TestVariable(self)
         result.name = variable_name
         return result
 
-    def check_variable_defined_for_entity(self, variable_name: str) -> bool:
-        return True
-
 
 class TestGroupEntity(GroupEntity):
     def get_variable(
         self,
         variable_name: str,
         check_existence: bool = False,
-    ) -> TestVariable:
+    ) -> None | TestVariable:
         result = TestVariable(self)
         result.name = variable_name
         return result
 
-    def check_variable_defined_for_entity(self, variable_name: str) -> bool:
-        return True
-
 
 @pytest.fixture
 def persons():

tests/web_api/test_calculate.py

@@ -52,15 +52,15 @@ def check_response(
         ),
         (
             '{"persons": {"bob": {"unknown_variable": {}}}}',
-            client.NOT_FOUND,
+            client.BAD_REQUEST,
             "persons/bob/unknown_variable",
-            "You tried to calculate or to set",
+            "You requested the variable 'unknown_variable', but it was not found in the entity 'persons'",
         ),
         (
             '{"persons": {"bob": {"housing_allowance": {}}}}',
             client.BAD_REQUEST,
             "persons/bob/housing_allowance",
-            "You tried to compute the variable 'housing_allowance' for the entity 'persons'",
+            "You requested the variable 'housing_allowance', but it was not found in the entity 'persons'",
         ),
         (
             '{"persons": {"bob": {"salary": 4000 }}}',