Skip to content

refactor: acled#218

Draft
nikbpetrov wants to merge 4 commits into
forecastingresearch:mainfrom
nikbpetrov:acled
Draft

refactor: acled#218
nikbpetrov wants to merge 4 commits into
forecastingresearch:mainfrom
nikbpetrov:acled

Conversation

@nikbpetrov

@nikbpetrov nikbpetrov commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator
  • note the requirements.txt change for base_eval - unavaoidable w/o a ton of additional hassle; comments explain why/what; will repeat for wikipedia
  • the repetition of AcledFetchFrame as well as the explicit dtype columns is smelly but can't be avoided unless we do a full refactor of acled
  • acled is the only source that requires email/passwords creds so these are private (c.f. _require_api_key() on the base class)
  • _io._read_acled_dfr() is refactored to reuse acled._prepare_resolution_data() (DRY); this means that resolve and base_eval/naive and dummy forecaster jobs both bring in the heavy acled deps
    • note that acled uses requests and numpy modules but both resolve/base eval bring those in implicitly (google-cloud-storage brings in requests, pandas brings in numpy) - this is behaviour not tied to this PR
  • note the @backoff.on_exception on _get_access_token behaviour change: instead of manual handling, we let backoff handle retries/erroring, matching other sources' behaviour

@nikbpetrov

Copy link
Copy Markdown
Collaborator Author

Parity test only on update job (not running fetch!): old vs new (main branch code vs pr code) is perfectly identical; both disagree partially (712 file diffs out of 3374) with prod but only because prod's computation of freeze_datetime_value depends on today and the last time acled ran was on June 10 (tests ran on June 13). That key, namely freeze_datetime_value is the only discrepancy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant