diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f3b8eef0..0e4e32e5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ # Changelog +# 44.0.0 [#1286](https://github.com/openfisca/openfisca-core/pull/1286) + +#### New features + +- Add `CoreEntity.variables` + - Allows for querying the variables defined for an entity. + +#### Deprecations + +- Remove `CoreEntity.check_role_validity` + - The method in itself had no relation to `entities` (it was a static + method, so no depending neither on an entity class nor an instance). It + could have been kept as a `populations` helper, where it was exclusively + used. Nonetheless, `check_role_validity` had the peculiar behaviour of + not failing but when the value of `role` is not `None`, that granted it + in the author's opinion the right to be removed so as to avoid confusion. +- Deprecate `class_override` arg from `entities.build_entity` + - That argument actually did nothing, and linters have been complaining + about this argument for almost 10 years now. It is only being marked as + a breaking change for respect to the semantic versioning protocol. +- Deprecate `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. +- Deprecate `CoreEntity.set_tax_benefit_system` + - The feature is now provided by simple variable assignment: + ```python + entity.tax_benefit_system = tax_benefit_system + ``` + - Likewise, `entity.tax_benefit_system` becomes part of the public API + (which implicitly it was as the private marker was being violated). + ### 43.2.5 [#1296](https://github.com/openfisca/openfisca-core/pull/1296) #### Bugfix diff --git a/openfisca_core/entities/__init__.py b/openfisca_core/entities/__init__.py index 1811e3fe9..32af3e0f0 100644 --- a/openfisca_core/entities/__init__.py +++ b/openfisca_core/entities/__init__.py @@ -2,13 +2,13 @@ from . import types from ._core_entity import CoreEntity +from ._errors import TaxBenefitSystemUnsetError, VariableNotFoundError from .entity import Entity from .group_entity import GroupEntity from .helpers import build_entity, find_role from .role import Role SingleEntity = Entity -check_role_validity = CoreEntity.check_role_validity __all__ = [ "CoreEntity", @@ -16,8 +16,9 @@ "GroupEntity", "Role", "SingleEntity", + "TaxBenefitSystemUnsetError", + "VariableNotFoundError", "build_entity", - "check_role_validity", "find_role", "types", ] diff --git a/openfisca_core/entities/_core_entity.py b/openfisca_core/entities/_core_entity.py index 33002e9af..2fae0d167 100644 --- a/openfisca_core/entities/_core_entity.py +++ b/openfisca_core/entities/_core_entity.py @@ -1,12 +1,12 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar import abc -import os from . import types as t -from .role import Role +from ._errors import TaxBenefitSystemUnsetError, VariableNotFoundError class CoreEntity: @@ -45,7 +45,7 @@ class CoreEntity: is_person: ClassVar[bool] #: A ``TaxBenefitSystem`` instance. - _tax_benefit_system: None | t.TaxBenefitSystem = None + tax_benefit_system: None | t.TaxBenefitSystem = None @abc.abstractmethod def __init__(self, *__args: object, **__kwargs: object) -> None: ... @@ -53,29 +53,16 @@ def __init__(self, *__args: object, **__kwargs: object) -> None: ... def __repr__(self) -> str: return f"{self.__class__.__name__}({self.key})" - def set_tax_benefit_system(self, tax_benefit_system: t.TaxBenefitSystem) -> None: - """A ``CoreEntity`` belongs to a ``TaxBenefitSystem``.""" - self._tax_benefit_system = tax_benefit_system - - def get_variable( - self, - variable_name: t.VariableName, - check_existence: bool = False, - ) -> t.Variable | None: - """Get ``variable_name`` from ``variables``. - - Args: - variable_name: The ``Variable`` to be found. - check_existence: Was the ``Variable`` found? + @property + def variables(self, /) -> Mapping[t.VariableName, t.Variable]: + """Get all variables defined for the entity. Returns: - Variable: When the ``Variable`` exists. - None: When the ``Variable`` doesn't exist. + dict[str, Variable]: The variables defined for the entity. Raises: - ValueError: When the :attr:`_tax_benefit_system` is not set yet. - ValueError: When ``check_existence`` is ``True`` and - the ``Variable`` doesn't exist. + TaxBenefitSystemUnsetError: When the :attr:`.tax_benefit_system` is + not set yet. Examples: >>> from openfisca_core import ( @@ -85,50 +72,69 @@ def get_variable( ... variables, ... ) - >>> this = entities.SingleEntity("this", "", "", "") - >>> that = entities.SingleEntity("that", "", "", "") + >>> this = entities.SingleEntity("this", "these", "", "") + >>> that = entities.SingleEntity("that", "those", "", "") - >>> this.get_variable("tax") + >>> this.variables Traceback (most recent call last): - ValueError: You must set 'tax_benefit_system' before calling this... + TaxBenefitSystemUnsetError: The tax and benefit system is not se... - >>> tax_benefit_system = taxbenefitsystems.TaxBenefitSystem([this]) - >>> this.set_tax_benefit_system(tax_benefit_system) + >>> tbs = taxbenefitsystems.TaxBenefitSystem([this, that]) + >>> this, that = tbs.entities - >>> this.get_variable("tax") + >>> this.variables + {} - >>> this.get_variable("tax", check_existence=True) - Traceback (most recent call last): - VariableNotFoundError: You tried to calculate or to set a value... + >>> that.variables + {} >>> class tax(variables.Variable): ... definition_period = periods.MONTH ... value_type = float ... entity = that - >>> this._tax_benefit_system.add_variable(tax) + >>> tbs.add_variable(tax) - >>> this.get_variable("tax") + >>> this.variables + {} + + >>> that.variables + {'tax': } + + >>> that.variables["tax"] """ - if self._tax_benefit_system is None: - msg = "You must set 'tax_benefit_system' before calling this method." - raise ValueError( - msg, - ) - return self._tax_benefit_system.get_variable(variable_name, check_existence) + if self.tax_benefit_system is None: + raise TaxBenefitSystemUnsetError + return { + name: variable + for name, variable in self.tax_benefit_system.variables.items() + if variable.entity.key == self.key + } - def check_variable_defined_for_entity(self, variable_name: t.VariableName) -> None: - """Check if ``variable_name`` is defined for ``self``. + def get_variable( + self, + variable_name: t.VariableName, + /, + check_existence: bool = False, + ) -> None | t.Variable: + """Get ``variable_name`` from ``variables``. Args: variable_name: The ``Variable`` to be found. + check_existence: Was the ``Variable`` found? + + Returns: + Variable: When the ``Variable`` exists. + None: When the ``Variable`` doesn't exist. Raises: - ValueError: When the ``Variable`` exists but is defined - for another ``Entity``. + TaxBenefitSystemUnsetError: When the :attr:`.tax_benefit_system` is + not set yet. + VariableNotFoundError: When ``check_existence`` is ``True`` and the + ``Variable`` doesn't exist. Examples: >>> from openfisca_core import ( @@ -138,81 +144,43 @@ def check_variable_defined_for_entity(self, variable_name: t.VariableName) -> No ... variables, ... ) - >>> this = entities.SingleEntity("this", "", "", "") - >>> that = entities.SingleEntity("that", "", "", "") - >>> tax_benefit_system = taxbenefitsystems.TaxBenefitSystem([that]) - >>> this.set_tax_benefit_system(tax_benefit_system) + >>> this = entities.SingleEntity("this", "these", "", "") + >>> that = entities.SingleEntity("that", "those", "", "") - >>> this.check_variable_defined_for_entity("tax") + >>> this.get_variable("tax") Traceback (most recent call last): - VariableNotFoundError: You tried to calculate or to set a value... + TaxBenefitSystemUnsetError: The tax and benefit system is not se... - >>> class tax(variables.Variable): - ... definition_period = periods.WEEK - ... value_type = int - ... entity = that + >>> tbs = taxbenefitsystems.TaxBenefitSystem([this, that]) + >>> this, that = tbs.entities - >>> this._tax_benefit_system.add_variable(tax) - + >>> this.get_variable("tax") - >>> this.check_variable_defined_for_entity("tax") + >>> this.get_variable("tax", check_existence=True) Traceback (most recent call last): - ValueError: You tried to compute the variable 'tax' for the enti... + VariableNotFoundError: You requested the variable 'tax', but it ... - >>> tax.entity = this + >>> class tax(variables.Variable): + ... definition_period = periods.MONTH + ... value_type = float + ... entity = that - >>> this._tax_benefit_system.update_variable(tax) + >>> tbs.add_variable(tax) - >>> 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:", - ".", - ) - raise ValueError(os.linesep.join(message)) - - @staticmethod - def check_role_validity(role: object) -> None: - """Check if ``role`` is an instance of ``Role``. - - Args: - role: Any object. - - Raises: - ValueError: When ``role`` is not a ``Role``. - - Examples: - >>> from openfisca_core import entities - - >>> role = entities.Role({"key": "key"}, object()) - >>> entities.check_role_validity(role) + >>> this.get_variable("tax") - >>> entities.check_role_validity("hey!") - Traceback (most recent call last): - ValueError: hey! is not a valid role + >>> that.get_variable("tax") + """ - if role is not None and not isinstance(role, Role): - msg = f"{role} is not a valid role" - raise ValueError(msg) + if self.tax_benefit_system is None: + raise TaxBenefitSystemUnsetError + 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"] diff --git a/openfisca_core/entities/_errors.py b/openfisca_core/entities/_errors.py new file mode 100644 index 000000000..fb223bf4d --- /dev/null +++ b/openfisca_core/entities/_errors.py @@ -0,0 +1,32 @@ +from . import types as t + + +class TaxBenefitSystemUnsetError(ValueError): + """Raised when a tax and benefit system is not set yet.""" + + def __init__(self) -> None: + msg = ( + "The tax and benefit system is not set yet. You need to set it " + "before using this method: `entity.tax_benefit_system = ...`." + ) + super().__init__(msg) + + +class VariableNotFoundError(ValueError): + """Raised when a requested variable is not defined for as entity.""" + + def __init__(self, name: t.VariableName, plural: t.EntityPlural) -> None: + msg = ( + f"You requested the variable '{name}', but it was not found in " + f"the entity '{plural}'. Are you sure you spelled '{name}' " + "correctly? If this code used to work and suddenly does not, " + "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:" + "." + ) + super().__init__(msg) + + +__all__ = ["TaxBenefitSystemUnsetError", "VariableNotFoundError"] diff --git a/openfisca_core/entities/helpers.py b/openfisca_core/entities/helpers.py index 1dcdad88a..36e90292c 100644 --- a/openfisca_core/entities/helpers.py +++ b/openfisca_core/entities/helpers.py @@ -15,7 +15,6 @@ def build_entity( roles: None | Sequence[t.RoleParams] = None, is_person: bool = False, *, - class_override: object = None, containing_entities: Sequence[str] = (), ) -> t.SingleEntity | t.GroupEntity: """Build an ``Entity`` or a ``GroupEntity``. @@ -27,7 +26,6 @@ def build_entity( doc: A full description. roles: A list of roles —if it's a ``GroupEntity``. is_person: If is an individual, or not. - class_override: ? containing_entities: Keys of contained entities. Returns: diff --git a/openfisca_core/populations/_core_population.py b/openfisca_core/populations/_core_population.py index 0041a6927..048fbd9f8 100644 --- a/openfisca_core/populations/_core_population.py +++ b/openfisca_core/populations/_core_population.py @@ -87,7 +87,7 @@ def __call__( >>> population("salary", period) >>> tbs = taxbenefitsystems.TaxBenefitSystem([person]) - >>> person.set_tax_benefit_system(tbs) + >>> person.tax_benefit_system = tbs >>> simulation = simulations.Simulation(tbs, {person.key: population}) >>> population("salary", period) Traceback (most recent call last): @@ -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): @@ -365,7 +365,7 @@ def get_holder(self, variable_name: t.VariableName) -> t.Holder: ... value_type = int >>> tbs = taxbenefitsystems.TaxBenefitSystem([person]) - >>> person.set_tax_benefit_system(tbs) + >>> person.tax_benefit_system = tbs >>> population = populations.SinglePopulation(person) >>> simulation = simulations.Simulation(tbs, {person.key: population}) >>> population.get_holder("income_tax") @@ -380,7 +380,7 @@ def get_holder(self, variable_name: t.VariableName) -> t.Holder: >> array([3500]) """ - self.entity.check_role_validity(role) + if role is not None and not isinstance(role, entities.Role): + msg = f"{role} is not a valid role" + raise TypeError(msg) self.members.check_array_compatible_with_entity(array) if role is not None: role_filter = self.members.has_role(role) @@ -140,7 +142,9 @@ def any(self, array, role=None): @projectors.projectable def reduce(self, array, reducer, neutral_element, role=None): self.members.check_array_compatible_with_entity(array) - self.entity.check_role_validity(role) + if role is not None and not isinstance(role, entities.Role): + msg = f"{role} is not a valid role" + raise TypeError(msg) position_in_entity = self.members_position role_filter = self.members.has_role(role) if role is not None else True filtered_array = numpy.where(role_filter, array, neutral_element) @@ -260,7 +264,9 @@ def value_from_person(self, array, role, default=0): The result is a vector which dimension is the number of entities """ - self.entity.check_role_validity(role) + if role is not None and not isinstance(role, entities.Role): + msg = f"{role} is not a valid role" + raise TypeError(msg) if role.max != 1: msg = f"You can only use value_from_person with a role that is unique in {self.key}. Role {role.key} is not unique." raise Exception( @@ -312,7 +318,9 @@ def value_from_first_person(self, array): def project(self, array, role=None): self.check_array_compatible_with_entity(array) - self.entity.check_role_validity(role) + if role is not None and not isinstance(role, entities.Role): + msg = f"{role} is not a valid role" + raise TypeError(msg) if role is None: return array[self.members_entity_id] role_condition = self.members.has_role(role) diff --git a/openfisca_core/populations/population.py b/openfisca_core/populations/population.py index 24742ab0a..97714aa16 100644 --- a/openfisca_core/populations/population.py +++ b/openfisca_core/populations/population.py @@ -2,7 +2,7 @@ import numpy -from openfisca_core import projectors +from openfisca_core import entities, projectors from . import types as t from ._core_population import CorePopulation @@ -49,7 +49,9 @@ def has_role(self, role: t.Role) -> None | t.BoolArray: if self.simulation is None: return None - self.entity.check_role_validity(role) + if role is not None and not isinstance(role, entities.Role): + msg = f"{role} is not a valid role" + raise TypeError(msg) group_population = self.simulation.get_population(role.entity.plural) @@ -68,7 +70,9 @@ def value_from_partner( role: t.Role, ) -> None | t.FloatArray: self.check_array_compatible_with_entity(array) - self.entity.check_role_validity(role) + if role is not None and not isinstance(role, entities.Role): + msg = f"{role} is not a valid role" + raise TypeError(msg) if not role.subroles or len(role.subroles) != 2: msg = "Projection to partner is only implemented for roles having exactly two subroles." diff --git a/openfisca_core/simulations/simulation_builder.py b/openfisca_core/simulations/simulation_builder.py index 7464b4650..6c4ab9fc8 100644 --- a/openfisca_core/simulations/simulation_builder.py +++ b/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] diff --git a/openfisca_core/taxbenefitsystems/tax_benefit_system.py b/openfisca_core/taxbenefitsystems/tax_benefit_system.py index 1c582e240..509435aa4 100644 --- a/openfisca_core/taxbenefitsystems/tax_benefit_system.py +++ b/openfisca_core/taxbenefitsystems/tax_benefit_system.py @@ -72,7 +72,7 @@ def __init__(self, entities: Sequence[Entity]) -> None: entity for entity in self.entities if not entity.is_person ] for entity in self.entities: - entity.set_tax_benefit_system(self) + entity.tax_benefit_system = self @property def base_tax_benefit_system(self): @@ -572,7 +572,7 @@ def clone(self): ): new_dict[key] = value for entity in new_dict["entities"]: - entity.set_tax_benefit_system(new) + entity.tax_benefit_system = new new_dict["parameters"] = self.parameters.clone() new_dict["_parameters_at_instant_cache"] = {} diff --git a/openfisca_core/types.py b/openfisca_core/types.py index 9c8105741..de0a62d62 100644 --- a/openfisca_core/types.py +++ b/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,20 +71,23 @@ class CoreEntity(Protocol): - key: EntityKey - plural: EntityPlural - - def check_role_validity(self, role: object, /) -> None: ... - 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: ... @@ -263,6 +266,8 @@ def get_population(self, plural: None | str, /) -> CorePopulation: ... class TaxBenefitSystem(Protocol): person_entity: SingleEntity + @property + def variables(self, /) -> dict[VariableName, Variable]: ... def get_variable( self, variable_name: VariableName, diff --git a/setup.py b/setup.py index b466b2407..80f45041d 100644 --- a/setup.py +++ b/setup.py @@ -70,7 +70,7 @@ setup( name="OpenFisca-Core", - version="43.2.5", + version="44.0.0", author="OpenFisca Team", author_email="contact@openfisca.org", classifiers=[ diff --git a/tests/core/tools/test_runner/test_yaml_runner.py b/tests/core/tools/test_runner/test_yaml_runner.py index 6a02d14ce..6fa710578 100644 --- a/tests/core/tools/test_runner/test_yaml_runner.py +++ b/tests/core/tools/test_runner/test_yaml_runner.py @@ -15,7 +15,7 @@ class TaxBenefitSystem: def __init__(self) -> None: self.variables = {"salary": TestVariable()} self.person_entity = Entity("person", "persons", None, "") - self.person_entity.set_tax_benefit_system(self) + self.person_entity.tax_benefit_system = self def get_package_metadata(self): return {"name": "Test", "version": "Test"} diff --git a/tests/fixtures/entities.py b/tests/fixtures/entities.py index 6670a68da..8f36b69e1 100644 --- a/tests/fixtures/entities.py +++ b/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(): diff --git a/tests/web_api/test_calculate.py b/tests/web_api/test_calculate.py index f84642fb3..d672cee29 100644 --- a/tests/web_api/test_calculate.py +++ b/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 }}}',