Closed caldwellst closed 2 months ago
Phew what a diff! We should definitely do some manual testing before merging to make sure there are no expected errors introduced (ie. remaining mismatches between country
and location
). Just left a couple clarifying questions.
Then that is pulled into the metadata.
What metadata do you mean?
Then that is pulled into the metadata.
Whoops, sorry! What I meant was that the actual coverage is done in the update_{indicator}.R
files, and that is pulled in in the update_country_info.R
file (now renamed update_location_info.R
. I'm working on some yaml scheduling that I''ll update here.
Alright, have tested this on all indicators and static update scripts, as well as listing, and we are successful! I think we can merge now because need to now change the whole design of the email and make final indicator changes ha...
Nice looks great!
haven't finished yet, but going to create a running list of some general stuff here so you will have some useful feedback tomorrow before I start
still getting this warning on when i run update_displacement_*.R
Warning message:
In st_centroid.sfc(x, of_largest_polygon = FALSE) :
st_centroid does not give correct centroids for longitude/latitude data
a quick "Find in files" (command
+ shift
+ F
) search did reveal a few more places where country is still used over locations. Perhaps double check and decide if they need to change
Haven't fully gone through, but LGTM! and i think merging will expedite troubleshooting/fixes/iteration
a quick "Find in files" (command + shift + F) search did reveal a few more places where country is still used over locations. Perhaps double check and decide if they need to change
Yep, good idea, removed a few more that came back when merging to main!
still getting this warning on when i run
update_displacement_*.R
Can you create an issue for this? Because there is no centroids call in there. We will hopefully catch anyway, but if you can show me will be easier to jump in and fix.
This is ready for review, but putting into draft as we still are awaiting language confirmation on a few things. Basically, need to put together this file that contains all the locations metadata including indicator coverage. However, at the same time realised that we need to move our naming of the countries we analyse from
country
tolocation
. This is mainly a backend thing and not often pushed out to the user, but will impact how people sign up to Mailchimp.Need to test how it performs, but will wait once I have final signup form language tomorrow. That is when we can better test performance.
Coverage update
The primary functionality updated is that we use
update_coverage()
in all of the indicator updates to ensure that we keep an up to date list of places covered for that indicator in Azure. Then that is pulled into the metadata. Maybe that needs to be put into a scheduled update.