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

Improve comms #17

Closed caldwellst closed 1 year ago

caldwellst commented 1 year ago

Will rebase after the explicit-loading branch has been pulled into main. This improves the IPC plot and makes a small change to the email text. Just review the plot_ipc.R and email.Rmd files. Putting as draft now and will change to full PR once rebased, but you can review whenever you want.

caldwellst commented 1 year ago

Alright, this is ready for review! I fixed some of the IPC and other alert NLP. Basically making sure it doesn't appear in the email if it's NA. I fixed the IPC plot. And made a few other changes to the text that should be simple! Let me know if any other questions.

caldwellst commented 1 year ago

Here's an example of the plot_ipc() output, FYI, so you don't have to manually re-run:

image
zackarno commented 1 year ago

The new plot is beautiful! This PR is taking a while to review since I haven't really done a "review" so far on this repo. Therefore, I am not going to attempt to review the whole repo now, but will relay some general relevant questions/comments that arise as I try to review this. The main general questions/comments got lumped together here: https://github.com/OCHA-DAP/pa-global-monitoring/pull/17#discussion_r1295021071

zackarno commented 1 year ago

Great yeah a lot of that was out of scope for this PR, but became somewhat relevant when I was trying to test the functionality of some of the code contained in the repo generally as well as this PR. Anyways makes sense to continue the interesting discussion in the new issue you opened as well as on our call tomorrow.

From my side, the PR is good to merge! nice work, and I will get to the additional PRs ASAP.