Hea 688/ignore activities without wealth category#152
Conversation
The Data, Data2 and Data3 worksheets that contain the Livelihood Actitivies contain blank columns separating the Wealth Categories (VP, P, etc.). Sometimes these blank columns contain data where a 0 value or a formula has been copied across the row. We can detect the separator column because it won't have a Wealth Category, and then ignore any Livelihood Activities we find in it.
There was a problem hiding this comment.
Pull Request Overview
This PR filters out livelihood activities without a wealth category to prevent processing invalid data from blank columns in BSS (Baseline Survey Sheet). The changes ensure that only valid livelihood activities with proper wealth group categorization are processed.
- Filter livelihood activities to require a wealth category component
- Add assertion to prevent linting warnings about potential None values
- Fix type annotations in lookup module for better type safety
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pipelines/assets/livelihood_activity.py | Adds filtering logic to exclude activities without wealth category and includes assertion for linting |
| apps/common/lookups.py | Updates type annotations and default values for better type safety |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| livelihood_activity | ||
| for livelihood_activity in livelihood_activities_for_strategy | ||
| if any( | ||
| if livelihood_activity["wealth_group"][2] # Make sure there is a Wealth Category |
There was a problem hiding this comment.
Using a magic number (index 2) to access the wealth category makes the code unclear. Consider adding a named constant or accessing by key name to make the intention explicit.
| # Assertion to prevent linting from complaining about possible None values | ||
| assert livelihood_strategy is not None, ( | ||
| "Found Livelihood Activities from row %s, but there is no Livelihood Strategy defined." % row | ||
| ) |
There was a problem hiding this comment.
Using assertions for runtime validation in production code is not recommended. Consider using a proper exception like ValueError or RuntimeError instead, as assertions can be disabled with -O flag.
| # Assertion to prevent linting from complaining about possible None values | |
| assert livelihood_strategy is not None, ( | |
| "Found Livelihood Activities from row %s, but there is no Livelihood Strategy defined." % row | |
| ) | |
| # Raise an exception to prevent possible None values for livelihood_strategy | |
| if livelihood_strategy is None: | |
| raise ValueError( | |
| "Found Livelihood Activities from row %s, but there is no Livelihood Strategy defined." % row | |
| ) |
| ] | ||
|
|
||
| # Assertion to prevent linting from complaining about possible None values | ||
| assert livelihood_strategy is not None, ( |
There was a problem hiding this comment.
@rhunwicks, this line appears to block some previously processed rows. For example, I checked cell Data!A59, which has empty values across the entire row for the camel milk label. I reviewed the runs for livelihood_activity_instances asset locally for the BSSes: ML01/02 and NG05, where the labels are recorded as "Camels' milk" or "Lait de chamelle".
No description provided.