OCHA-DAP / ripc

Download and Tidy IPC and CH Data
GNU General Public License v3.0
2 stars 1 forks source link

hotfix on type-conversion of percentage columns #8

Closed zackarno closed 8 months ago

zackarno commented 9 months ago

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

Some _percentage columns reading as "character" some reading as "numeric". Issue outlined here:

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

I posed a few potential ideas for more robust fix's there, but have done a quick fix here to deal with the _percentage columns only to minimize any side-effects.

caldwellst commented 8 months ago

Decided to take the different approach. Given there were a lot of changes, I created a new branch. As well, note that you're looping through the lists a second time to make the changes. The cleaner approach would be to just change the parsing function in the initial looping during unnest().

zackarno commented 8 months ago

Cool, sounds good - I figured you would more easily come up w/ a fix that would make more sense.

I was wondering if you had any idea on best practices w/ regard adding tests for these kinds of issues. Would be nice if any issues cause by ipc api changes were more quickly flagged for trouble shooting. For our purposes this could be done on the HDXSignals side, but for larger audience/maintenance of {ripc} it might make sense somehow here.

caldwellst commented 8 months ago

Yeah for sure. I guess the issue is how to do tests without requiring certain columns around? I think maybe the test is just that the API call doesn't fail, so it's flagged if breaking changes occur? Or even we could save the outputs now, and if the API returns something different in terms of # and names of columns, we flag? Have any experience with that? I agree that we should do it on {ripc}.