Somalia/ipc #22

p-phung · 2025-10-07T09:33:59Z

new extractor for IPC acute food insecurity
define config for IPC data request
extractor also take cares of getting the right metadata for the IPC food insecurity
update readme accordingly

Todo:

proper way to load secrets
add IPC api key loading to the extractor

TjebbeH

Nice work!

I think main suggestion is to fail fast when things are unexpected rather than passing on None values or empty dataframes which then later have to be checked if they are None/empty. And quite some of the complexity of the code can be reduced by validating a bit stronger/earlier.

Let me know if you have any questions about my comments :)

retrievalpipeline/extract/extractors/ipc.py

TjebbeH · 2025-10-11T06:34:20Z

retrievalpipeline/extract/extractors/ipc.py

+load_dotenv()
+
+
+class IpcPropertiesSchema(BaseModel):


Many fields are optional and are allowed to be None, is this what you want? Is data were all these fields are None valid? Downside of allowing None's is that then later in the code you have to keep checking if things are None or not. This often leads to difficult to read code with many if statements. So it's advised to keep the locations where you allow None to a minimum and directly raise an error when things are not as the are expected to be.

I would suggets to think about if you really need these fields. If not remove them. If you do need them you probably need them to be something else then None?

I am aware of it. The data extracted from the API is pretty messy. There are nested geometry features in those features with from_date and to_date None. Only after exploding the nested features, I can extract the right data from there.

I wonder what else we could do to refine it from the tap like here?

retrievalpipeline/extract/extractors/ipc.py

TjebbeH · 2025-10-11T07:15:29Z

retrievalpipeline/extract/extractors/ipc.py

+            if geom_dict.get("type") == "GeometryCollection" and geom_dict.get("coordinates") is None:
+                geom_dict.pop("coordinates", None)
+
+            # skip invalid geometry


I dont know exactly what a geometry should look like, but can invalid geomeries not be filtered out by means of the pydantic schema's above?

the nested features issue seems to be quite random. And there are too many features to go through (Somalia has about 3500 features after filtered). Thats why I couldn't structure it in the pydantic schema and opt for "get first, filter later"

TjebbeH · 2025-10-11T07:16:02Z

retrievalpipeline/extract/extractors/ipc.py

+            geom_dict = copy.deepcopy(feature["geometry"])
+
+            # fix invalid GeometryCollection: remove "coordinates": None
+            if geom_dict.get("type") == "GeometryCollection" and geom_dict.get("coordinates") is None:


This is a sympthom of allowing None's

this is one of the issues caused by the messy nested features problem above. Happy to brainstorm about this to find another solution

TjebbeH · 2025-10-11T07:31:35Z

The integration test fails on retrieving env var, when you add @pytest.mark.needs_secrets, it will retrieve the env vars locally and skip the test in the CI. What was your idea to mock the api in the integration test?

p-phung

many of the issue causes by lack of type check for IpcGeometrySchema. I have added this type check Literal as you suggests them and it seem those issues are gone.

still one of the major issue is the input data from IPC api is quite messy with lots of random nested features that makes it difficult to do pydantic check. If you have some advice, happy to learn about it

p-phung · 2025-10-16T15:59:22Z

retrievalpipeline/extract/extractors/ipc.py

+load_dotenv()
+
+
+class IpcPropertiesSchema(BaseModel):


I am aware of it. The data extracted from the API is pretty messy. There are nested geometry features in those features with from_date and to_date None. Only after exploding the nested features, I can extract the right data from there.

I wonder what else we could do to refine it from the tap like here?

retrievalpipeline/extract/extractors/ipc.py

p-phung · 2025-10-23T08:09:42Z

retrievalpipeline/extract/extractors/ipc.py

+
+    id: str = Field(description="Identifier of data.", examples=["12511433"])
+    title: str = Field(description="Title of data.", examples=["Somalia - Acute Food Insecurity July 2017"])
+    link: str | None = Field(


this is not actually needed. Should I make it optional instead?

p-phung · 2025-10-23T08:10:18Z

retrievalpipeline/extract/extractors/ipc.py

+        description="Type of IPC Analysis (condition).",
+        examples=["A", "C"],
+    )
+    created: str = Field(description="Creation date of IPC data.", examples=["2018-06-04"])


retrievalpipeline/extract/extractors/ipc.py

p-phung · 2025-10-23T08:38:26Z

retrievalpipeline/extract/extractors/ipc.py

+            geom_dict = copy.deepcopy(feature["geometry"])
+
+            # fix invalid GeometryCollection: remove "coordinates": None
+            if geom_dict.get("type") == "GeometryCollection" and geom_dict.get("coordinates") is None:


this is one of the issues caused by the messy nested features problem above. Happy to brainstorm about this to find another solution

p-phung · 2025-10-23T08:41:24Z

retrievalpipeline/extract/extractors/ipc.py

+            if geom_dict.get("type") == "GeometryCollection" and geom_dict.get("coordinates") is None:
+                geom_dict.pop("coordinates", None)
+
+            # skip invalid geometry


the nested features issue seems to be quite random. And there are too many features to go through (Somalia has about 3500 features after filtered). Thats why I couldn't structure it in the pydantic schema and opt for "get first, filter later"

retrievalpipeline/extract/extractors/ipc.py

TjebbeH · 2025-10-24T06:56:29Z

retrievalpipeline/extract/extractors/ipc.py

+            except Exception as e:
+                logger.error(f"Validation failed for {iso2}: {e}")
+
+        return gdf_iso2


gdf_iso2 does not exist if data is none or if an exception is caught in the try explode geometries.

retrievalpipeline/extract/extractors/ipc.py

Tjebbe Hepkema and others added 30 commits October 7, 2025 11:23

update azure blob storage

df98b0b

allow multiple downloads that reuse session

aeafc4c

add worldpop extractor

01a5bc2

pandas and pydantic deps

3edb72b

tmp rename

7a155ac

update fixture with AgeStructureConfig

35a1ef4

update fixture age_structure_extractor

62dbcae

update urls test

0cfbdf5

print test cov to terminal

4655e3e

api json extractor tests

6b7fe62

update log for validation

e3fd0b8

reuse download_with_session

a05e699

retry downloads

dd56221

rm azure unspecific test

c1a5d82

dont sleep after last try

baf73cc

make retries optional

75f7d34

typehints retry

06200c1

typehints retry 2

8db6d6d

worldpop somalia

84a5736

use pytest tmp_path fixture

e660891

load .env for tests with marker needs_secrets

3f35b84

put ecmwf url in file for constants

aaf0c5a

update .env.example

6925381

update docstring

cfc4793

update docstring

9354e56

fix 1 off error number of retries

038ffeb

update docstring

de97e7d

add geopandas package

29598b9

add exception for ipc

5ab161b

tests for ipc extractor

b4b8399

p-phung added 3 commits October 10, 2025 12:26

add extractor name

3094836

replace api key from env

356756f

add extractor name to tests

c7261d4

TjebbeH requested changes Oct 11, 2025

View reviewed changes

p-phung added 2 commits October 16, 2025 13:31

rm load_dotenv

7fdaed3

break down gdf selection

ea9e598

p-phung commented Oct 23, 2025

View reviewed changes

p-phung added 8 commits October 23, 2025 13:57

update pydantic types for schemas

8551df2

format date fields in schemas

0820a33

rm return empty geodataframe in functions

257f8d4

add needs_secrets mark for pytest

bbfdc80

correct filter data by type

7852f42

change created date type of test meta schema

0ea1f8a

split test select data to select by year select by type

8b9a544

adapt expected results to test split

abdea27

p-phung marked this pull request as ready for review October 23, 2025 13:51

add needs_secret mark for pytest

7437151

TjebbeH reviewed Oct 24, 2025

View reviewed changes

retrievalpipeline/extract/extractors/ipc.py Show resolved Hide resolved

TjebbeH reviewed Oct 24, 2025

View reviewed changes

retrievalpipeline/extract/extractors/ipc.py Show resolved Hide resolved

p-phung added 8 commits October 24, 2025 11:22

Merge branch 'somalia/dev' into somalia/ipc and fix conflicts

819e63d

add ipc_config method

5030320

rm try/except

1a640df

rm return empty df

a84603c

cleanup

93d4e0a

fix lock file based on updated pyproject

3a886fe

correct ipc url

720f431

update and clean integration ipc test

ef1a848

p-phung merged commit d8f6c86 into somalia/dev Oct 24, 2025
1 check passed

		load_dotenv()


		class IpcPropertiesSchema(BaseModel):

		load_dotenv()


		class IpcPropertiesSchema(BaseModel):

Somalia/ipc #22

Somalia/ipc #22

Uh oh!

Conversation

p-phung commented Oct 7, 2025

Uh oh!

TjebbeH left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TjebbeH commented Oct 11, 2025

Uh oh!

p-phung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TjebbeH left a comment •

edited

Loading