Merge Variable and Column

[fpagnoux]

Breaking changes

Change the way Variables are declared

• Replace column attribute by value_type
  • Possible values of value_type are:
    • int
    • float
    • bool
    • str
    • date
    • Enum

Before:

class basic_income(Variable):
    column = FloatCol
    entity = Person
    definition_period = MONTH
    label = "Basic income provided to adults"

Now:

class basic_income(Variable):
    value_type = float
    entity = Person
    definition_period = MONTH
    label = "Basic income provided to adults"

• default is now a Variable attribute

Before:

class is_citizen(Variable):
    column = BoolCol(default = True)
    entity = Person
    definition_period = MONTH
    label = "Whether the person is a citizen"

Now:

class is_citizen(Variable):
    value_type = bool
    default_value = True
    entity = Person
    definition_period = MONTH
    label = "Whether the person is a citizen"

• For Variables which value_type is str, max_lentgh is now an attribute

Before:

class zipcode(Variable):
    column = FixedStrCol(max_length = 5)
    entity = Menage
    label = u"Code INSEE (depcom) du lieu de résidence"
    definition_period = MONTH

After:

class zipcode(Variable):
    value_type = str
    max_length = 5
    entity = Menage
    label = u"Code INSEE (depcom) du lieu de résidence"
    definition_period = MONTH

• For Variables which value_type is Enum, possible_values is now an attribute:

Before:

class housing_occupancy_status(Variable):
    column = EnumCol(
        enum = Enum([
            u'Tenant',
            u'Owner',
            u'Free logder',
            u'Homeless'])
        )
    entity = Household
    definition_period = MONTH
    label = u"Legal housing situation of the household concerning their main residence"

After:

class housing_occupancy_status(Variable):
    value_type = Enum
    possible_values = Enum([
        u'Tenant',
        u'Owner',
        u'Free logder',
        u'Homeless'
        ])
    entity = Household
    definition_period = MONTH
    label = u"Legal housing situation of the household concerning their main residence"

• Remove PeriodSizeIndependentIntCol:
  • Replaced by Variable attribute is_period_size_independent

Deprecate Column

Column are now considered deprecated. Preferably use Variable instead.

If you do need a column for retro-compatibility, you can use:

from openfisca_core.columns import make_column_from_variable

column = make_column_from_variable(variable)

• In TaxBenefitSystem:

  • Remove neutralize_column (deprecated since 9.1.0, replaced by neutralize_variable)
  • Rename column_by_name to variables
  • Rename get_column to get_variable
  • Remove update_column
  • Remove add_column
  • Remove automatically_loaded_variable (irrelevant since conversion columns have been removed)
  • Move VariableNotFound to errors module

• In Holder:

  • Rename holder.column to holder.variable

• In Column:

  • Column should only be instantiated using make_column_from_variable. Former constructors do not work anymore.
  • Remove column.start, which was None since 14.0.0
  • Replace column.formula_class by variable.formula
  • Replace column.enum by variable.possible_values
  • Replace column.default by variable.default_value

• In formulas:

  • Rename get_neutralized_column to get_neutralized_variable
  • Remove new_filled_column

• In Variable:

  • Remove to_column
  • Variables can now directly be instanciated:

class salary(Variable):
    entity = Person
    ...


salary_variable = salary()

[Anna-Livia]

Looks good but I have a few question on the remaining Column references

[Anna-Livia]

Would it be more relevant to have the Enum be outside of the Variable ?

[fpagnoux]

I don't know.

The goal of this part of the Changelog is to show OpenFisca reusers what has changed and what they need to do to adapt their code.

Moving the Enum declaration out of the variable definition is not related to this PR. It is more a consequence of #589. In France, most (all?) enums are declared in the variable block. I'm not sure we should anticipate the outcome of another PR in this PR changelog, at the risk of making this PR syntactic changes less understandable.

[Anna-Livia]

default_value

[Anna-Livia]

Why keep colums if they are deprecated ? Should there be a comment indicating that they are ?

[fpagnoux]

👍 , a comment is indeed necessary.

[Anna-Livia]

Why keep column ? Is the name still relevant ? If columns are deprecated, should we indicate it in the code ?

[fpagnoux]

Indicated it in columns.py.

This part of the code is full of Byriani, not properly documented, and we're not sure about what it does exactly. From there, two strategies are possible when we want to publish a change that impact this code:

• We try to understand how this piece of code work. When we succeed, we adapt the code so that it can work with the a Variable instead of a Column.
  • Advantage: cleaner once its one, if we don't pollute the new Variable class with stuff from Column that we don't really want to keep.
  • Drawback: high risk of getting lost, and to spend more time migrating this module we don't even know than making the changes we want to do.
• We consider this piece of code as legacy that we are maintaining in a minimalistic way. We try to avoid modifying it. If it needs a column, we give it a column, hoping that one day it will be rebuilt or remove.
  • Advantage: Our time investment is more focused. We are more likely to deliver in time.
  • Drawback: Some legacy modules stay in the code

I chose strategy 2, as columns were needed in several part of the code I don't want to deal with.

[Anna-Livia]

Would a comment here be useful ? As to what this method does ?

[fpagnoux]

Added a comment 👌

[Anna-Livia]

V20

[Anna-Livia]

V20