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

`recursive_filter_country()` - equality question #53

Closed zackarno closed 6 months ago

zackarno commented 6 months ago

Why is this equality alert_level_numeric <= current_level using <= rather than <

https://github.com/OCHA-DAP/hdx-signals/blob/b4de37752e9cb1e2f0d53c544e41d4c33a5c4676/src/alerts/filter_alerts.R#L152

It seems to me that say we have a high alert and and then another 181 days later we have another high alert, the second one should also be flagged?

caldwellst commented 6 months ago

The <= is there because we are saying filter out anything that is within 180 days of the current alert (current_date) and that has an alert level at or below the current alert level. So, if we only had alert level 1, we would still generate a new alert for a 2 within the next 180 days, but not if we already had a 2. If another 2 occurs after 181 days, then we would alert, yes.

zackarno commented 6 months ago

ah ok clear now. To make more clear, i'd think about changing the name to something like:

remove_subsequent_lte_alerts()

just because im not sure if it's actually using recursion and also filter() is non-directional in-terms of filter-out vs filter-to which i think is a bit confusing in this specific context

feel free to take it or leave it of course

caldwellst commented 6 months ago

lol I think it's not recursion, you're right. I think I called it that maybe when I thought I would make it truly recursive, but never did ha. I named it remove_subsequent_alerts(). Thanks!