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

quick-fix for ipc_get type conversion error #42

Closed zackarno closed 8 months ago

zackarno commented 8 months ago

Temporary hotfix to deal w/ improper type conversion from {ripc}.

Some _percentage columns reading as "character" some reading as "numeric" so they can't be bound together.

https://github.com/OCHA-DAP/ripc/issues/7#issue-2154372435

and I've also done a PR on {ripc} which implements the same quick fix. Nonetheless I recommend we go w/ the internal fix here as a temporary solution for several reasons which I believe all have the same goals of using time most efficiently and reducing potential side-effects:

  1. Updating {ripc} would require fast push's to main on 2 separate repo's without Seths review
  2. I have a quick solution here that works in {ripc}, but think we can come up w/ a more robust solution for {ripc}
  3. ripc is on CRAN, this push would just go to GH and not sure what would need to be done move it to CRAN
  4. reduce tangling w/ {renv} in signals -- it's can be a pain, especially when dealing w/ interactions of CRAN + GH packages + GH Actions.
  5. minimize chance of side-effects here or with other work.

You can see a couple failed runs in the GHA history log, the latest manual run on this branch worked, so I think it makes sense to push as a temporary solution to get it back on a CRON schedule.

zackarno commented 8 months ago

FYI for this temporary fix I just copied the relevant code from the {ripc} package to src/utils/ipc_get.R and adjusted the one small sections

from:

https://github.com/OCHA-DAP/ripc/blob/bccc53c780639f2100cf214365f5741a14055db6/R/ipc_get.R#L65-L70

to: https://github.com/OCHA-DAP/hdx-signals/blob/c0fd25a3481c300cf6c84a8a28834e8d00642df0/src/utils/ipc_get.R#L431-L453