openfisca / openfisca-core

OpenFisca core engine. See other repositories for countries-specific code & data.
https://openfisca.org
GNU Affero General Public License v3.0
168 stars 75 forks source link

Improve group entity creation and navigation API #1091

Open JasonMorrisSC opened 2 years ago

JasonMorrisSC commented 2 years ago

Entities are used to represent things in the real world. Right now the parameter is_person is being used to distinguish between entities that can "contain" other entities (GroupEntities) and entities that can't.

Using is_person causes the user to think that "Person" is the only valid non-group Entity, which isn't true. It also causes the user to think that the only type of entity that can be contained is a Person, which isn't true.

You could just rename it, but there is also a problem that the required parameters for build_entity() aren't clear from the tooltips in the IDE, because it is being used for both entities and groups, and the things that are mandatory for groups are listed as optional for the function.

I would suggest instead adding a build_group_entity() function, make the role and containing_objects parameters mandatory, remove is_person from both, and set is_individual inside the function and use that term everywhere else in the code, and updating the documentation accordingly. is_person could return is_individual for backward compatibility.

Consider this a placeholder for a future pull request to that effect, or feel free to beat me to it.

MattiSG commented 2 years ago

Thanks @JasonMorrisSC! Correcting the wording from “person” to something more generic sounds appropriate indeed to decrease confusion 👍 However, “individual” still sounds to my non-native English speaking ears still quite close to a human individual. Such an entity is used as a pivot in the modelled system, to navigate between entities. For example, @sandcha manipulates companies in https://github.com/openfisca/openfisca-france-fiscalite-miniere as the base “individual” entity.

Would you see a name that could carry such a meaning while maintaining its understandability in the most common case where this type is Person? 🙂