search

OCHA-DAP / hdx-signals

HDX Signals
https://un-ocha-centre-for-humanitarian.gitbook.io/hdx-signals/
GNU General Public License v3.0
6 stars 0 forks source link

Fix: adjust code for not recognized ison codes #260

Closed martinig94 closed 1 week ago

martinig94 commented 1 week ago

Filtering out unrecognized iso3 codes which are for now hardcoded in a vector. Ideally these could be stored in a table of a database

zackarno commented 1 week ago

looks like a nice workaround patch, but currently getting this error:

df_wrangled <- wrangle_conflict$wrangle(df_raw)
#> Error in `reframe()`:
#> ℹ In argument: `complete(data = pick(everything()), ..., fill = fill, explicit = explicit)`.
#> ℹ In group 236: `iso3 = NA` and `acled_hdx_url = NA`.
#> Caused by error in `seq.int()`:
#> ! 'to' must be a finite number
#> Run `rlang::last_trace()` to see where the error occurred.
#> Warning message:
#> There was 1 warning in `dplyr$mutate()`.
#> ℹ In argument: `iso3 = location_codes$ison_to_iso3(as.numeric(iso))`.
#> Caused by warning:
#> ! Some values were not matched unambiguously: 2
zackarno commented 1 week ago

looks like a nice workaround patch, but currently getting this error:

df_wrangled <- wrangle_conflict$wrangle(df_raw)
#> Error in `reframe()`:
#> ℹ In argument: `complete(data = pick(everything()), ..., fill = fill, explicit = explicit)`.
#> ℹ In group 236: `iso3 = NA` and `acled_hdx_url = NA`.
#> Caused by error in `seq.int()`:
#> ! 'to' must be a finite number
#> Run `rlang::last_trace()` to see where the error occurred.
#> Warning message:
#> There was 1 warning in `dplyr$mutate()`.
#> ℹ In argument: `iso3 = location_codes$ison_to_iso3(as.numeric(iso))`.
#> Caused by warning:
#> ! Some values were not matched unambiguously: 2

wait! i just restarted my R-session and the error went away, my bad! classic...

martinig94 commented 1 week ago

Ah that's interesting because from this it looked like there is no isocode for the area. Will have a better look!

Okay looks great. I say just add one descriptive comment as suggested in #260 (comment) and go ahead and merge it so the patch is up and running on main.

We may wan't to refactor later and deal with slightly differently, but for now it should be fine. I'd say the next step which could be a small PR would just be to treat it the same as I highlighted in #260 (comment)

this would require us to create an iso3 code however. Perhaps we could just go with SBA as shown in this Wikipedia article for Akrotiri and Dhekelia