From c934b0a925c73ed76c13807f6361362b51fb41e2 Mon Sep 17 00:00:00 2001 From: Roger Hunwicks Date: Fri, 22 Nov 2024 16:04:40 -0500 Subject: [PATCH 1/2] Fix incorrect bss_sheet attribute - see HEA-572 --- pipelines/assets/livelihood_activity.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pipelines/assets/livelihood_activity.py b/pipelines/assets/livelihood_activity.py index 93b7e19..b42c4ea 100644 --- a/pipelines/assets/livelihood_activity.py +++ b/pipelines/assets/livelihood_activity.py @@ -478,7 +478,7 @@ def get_instances_from_dataframe( previous_livelihood_activities_for_strategy[i]["milking_animals"] ) - # Calculated kcals_consumed if the livelihood activity only contains the percentage_kcals. + # Calculate kcals_consumed if the livelihood activity only contains the percentage_kcals. # This is typical for ButterProduction and consumption of green crops. # Derive it by multiplying percentage_kcals by: # 2100 (kcals per person per day) * 365 (days per year) * average_household_size (from Row 40) @@ -763,7 +763,7 @@ def get_instances_from_dataframe( "scenario": LivelihoodActivityScenario.BASELINE, "wealth_group": wealth_group_df.iloc[i]["natural_key"], # Include the column and row to aid trouble-shooting - "bss_sheet": "Data", + "bss_sheet": worksheet_name, "bss_column": df.columns[i + 1], "bss_row": row, "activity_label": label, From 67a8fcae2ce6adc82e769352f3e35b4a3b76474a Mon Sep 17 00:00:00 2001 From: Roger Hunwicks Date: Fri, 22 Nov 2024 18:11:03 -0500 Subject: [PATCH 2/2] Correct payment_per_time field definition - see HEA-572 payment_per_time was defined as a PositiveSmallIntegerField, but some data contains values to large for a small integer, and it is possible that for OtherCashIncome the payment_per_time could be a non-integer. Therefore, change to a FloatField with a validator to ensure the value is positive. --- ...alter_livelihoodactivity_price_and_more.py | 48 +++++++++++++++++++ apps/baseline/models.py | 40 ++++++++++++++-- apps/common/models.py | 11 +++++ pipelines/assets/livelihood_activity.py | 3 +- 4 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 apps/baseline/migrations/0018_alter_livelihoodactivity_price_and_more.py diff --git a/apps/baseline/migrations/0018_alter_livelihoodactivity_price_and_more.py b/apps/baseline/migrations/0018_alter_livelihoodactivity_price_and_more.py new file mode 100644 index 0000000..df39e8d --- /dev/null +++ b/apps/baseline/migrations/0018_alter_livelihoodactivity_price_and_more.py @@ -0,0 +1,48 @@ +# Generated by Django 5.1.1 on 2024-11-22 21:21 + +from django.db import migrations, models + +import common.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("baseline", "0017_alter_livelihoodactivity_percentage_kcals"), + ] + + operations = [ + migrations.AlterField( + model_name="livelihoodactivity", + name="price", + field=models.FloatField( + blank=True, + help_text="Price per unit", + null=True, + validators=[common.models.validate_positive], + verbose_name="Price", + ), + ), + migrations.AlterField( + model_name="othercashincome", + name="payment_per_time", + field=models.FloatField( + blank=True, + help_text="Amount of money received each time the labor is performed", + null=True, + validators=[common.models.validate_positive], + verbose_name="Payment per time", + ), + ), + migrations.AlterField( + model_name="paymentinkind", + name="payment_per_time", + field=models.FloatField( + blank=True, + help_text="Amount of item received each time the labor is performed", + null=True, + validators=[common.models.validate_positive], + verbose_name="Payment per time", + ), + ), + ] diff --git a/apps/baseline/models.py b/apps/baseline/models.py index 03ef2ca..92a8f67 100644 --- a/apps/baseline/models.py +++ b/apps/baseline/models.py @@ -1102,7 +1102,13 @@ class LivelihoodActivity(common_models.Model): # but there are exceptions, such as MilkProduction, where there is also an amount used for ButterProduction quantity_consumed = models.PositiveIntegerField(blank=True, null=True, verbose_name=_("Quantity Consumed")) - price = models.FloatField(blank=True, null=True, verbose_name=_("Price"), help_text=_("Price per unit")) + price = models.FloatField( + blank=True, + null=True, + validators=[common_models.validate_positive], + verbose_name=_("Price"), + help_text=_("Price per unit"), + ) # Can be calculated / validated as `quantity_sold * price` for livelihood strategies that involve the sale of # a proportion of the household's own production. income = models.FloatField(blank=True, null=True, help_text=_("Income")) @@ -1584,8 +1590,12 @@ class PaymentInKind(LivelihoodActivity): """ # Production calculation/validation is `people_per_household * times_per_month * months_per_year` - payment_per_time = models.PositiveSmallIntegerField( - verbose_name=_("Payment per time"), help_text=_("Amount of item received each time the labor is performed") + payment_per_time = models.FloatField( + blank=True, + null=True, # Not required if people_per_household or times_per_month is null + validators=[common_models.validate_positive], + verbose_name=_("Payment per time"), + help_text=_("Amount of item received each time the labor is performed"), ) people_per_household = models.PositiveSmallIntegerField( verbose_name=_("People per household"), help_text=_("Number of household members who perform the labor") @@ -1601,6 +1611,14 @@ class PaymentInKind(LivelihoodActivity): help_text=_("Number of times in a year that the labor is performed"), ) + def clean(self): + # payment_per_time is only required if people_per_household and times_per_month are provided + if self.people_per_household and self.times_per_month and not self.payment_per_time: + raise ValidationError( + _("Payment per time is required if people per household and times per month are provided") + ) + super().clean() + def validate_quantity_produced(self): if ( self.quantity_produced @@ -1711,8 +1729,12 @@ class OtherCashIncome(LivelihoodActivity): # However, some other income (e.g. Remittances) just has a number of times per year and is not calculated from # people_per_household, etc. Therefore those fields must be nullable, and we must store the total number of times # per year as a separate field - payment_per_time = models.PositiveSmallIntegerField( - verbose_name=_("Payment per time"), help_text=_("Amount of money received each time the labor is performed") + payment_per_time = models.FloatField( + blank=True, + null=True, # Not required if people_per_household or times_per_month is null + validators=[common_models.validate_positive], + verbose_name=_("Payment per time"), + help_text=_("Amount of money received each time the labor is performed"), ) people_per_household = models.PositiveSmallIntegerField( verbose_name=_("People per household"), @@ -1734,6 +1756,14 @@ class OtherCashIncome(LivelihoodActivity): help_text=_("Number of times in a year that the income is received"), ) + def clean(self): + # payment_per_time is only required if people_per_household and times_per_month are provided + if self.people_per_household and self.times_per_month and not self.payment_per_time: + raise ValidationError( + _("Payment per time is required if people per household and times per month are provided") + ) + super().clean() + def validate_income(self): if ( self.people_per_household diff --git a/apps/common/models.py b/apps/common/models.py index c19e0af..30f4ceb 100644 --- a/apps/common/models.py +++ b/apps/common/models.py @@ -32,6 +32,17 @@ logger = logging.getLogger(__name__) +def validate_positive(value): + """ + Validator to ensure that a value is positive. + + The normal models.validators.MinValueValidator(0) isn't suitable because it allows zero, + which isn't appropriate for some attributes, such as price. + """ + if value < 0: + raise ValidationError(_("Value must be greater than 0.")) + + class ShowQueryVariablesMixin(object): """ Mixin for models.Manager classes that shows the query arguments when a get() query fails diff --git a/pipelines/assets/livelihood_activity.py b/pipelines/assets/livelihood_activity.py index b42c4ea..fb96f02 100644 --- a/pipelines/assets/livelihood_activity.py +++ b/pipelines/assets/livelihood_activity.py @@ -520,7 +520,8 @@ def get_instances_from_dataframe( # that is embedded in the ButterProduction calculations in current BSSs livelihood_activity["type_of_milk_consumed"] = MilkProduction.MilkType.WHOLE - # Add the `times_per_year` to FoodPurchase, because it is not present in any current BSS + # Add the `times_per_year` to FoodPurchase, PaymentInKind and OtherCashIncome, + # because it is not in the current BSSs if ( "times_per_month" in livelihood_strategy["attribute_rows"] and "months_per_year" in livelihood_strategy["attribute_rows"]