I don't really like how the map testing code is structured and could be improved by adjusting how the utils/images/save_image.R functions work and the email/mailchimp/images.R functions work. However, it should sit outside the #75 because there's a lot of changes necessary.
In general we should do something like:
Remove the gg$ggsave() set of code from images.R and let those be agnostic to image type, so removing mc_upload_plot() basically and just keeping mc_upload_image().
Adjust the save_image.R functionality to not always upload to Mailchimp. Turn save_image() into save_image_mc().
Then create a new save_image() where p, iso3, width, and height are required, and the other arguments are now defaults to NULL like indicator_id and date. save_map() is the same as now basically, just mirroring the new save_image().
The first thing done in this should be to save out the image to a tempfile, then we can figure out where it gets saved.
Add a new argument, something like output_location that defaults to mailchimp, in which case it uses save_image_mc() to upload to Mailchimp, our default behaviour most of the time. This would ensure that current calls to save_image() and save_map() require limited changes.
If output_location is azure, then we also expect output_dir, a {glue} string with iso3 defined for uploading images to Azure for testing. So, "test/maps/{iso3}.png" for instance. We have save_image_az() called to handle that.
If output_location is local, then we also expect output_dir as above, and create a save_image_local() function for this.
In this way, we can default to saving to Mailchimp, but have capacity to save to Azure as default in our test/manual/maps directory for instance, but also able to save locally for iterative development if we are interested.
I don't really like how the map testing code is structured and could be improved by adjusting how the
utils/images/save_image.R
functions work and theemail/mailchimp/images.R
functions work. However, it should sit outside the #75 because there's a lot of changes necessary.In general we should do something like:
gg$ggsave()
set of code fromimages.R
and let those be agnostic to image type, so removingmc_upload_plot()
basically and just keepingmc_upload_image()
.save_image.R
functionality to not always upload to Mailchimp. Turnsave_image()
intosave_image_mc()
.save_image()
wherep
,iso3
,width
, andheight
are required, and the other arguments are now defaults toNULL
likeindicator_id
anddate
.save_map()
is the same as now basically, just mirroring the newsave_image()
.output_location
that defaults tomailchimp
, in which case it usessave_image_mc()
to upload to Mailchimp, our default behaviour most of the time. This would ensure that current calls tosave_image()
andsave_map()
require limited changes.output_location
isazure
, then we also expectoutput_dir
, a{glue}
string withiso3
defined for uploading images to Azure for testing. So, "test/maps/{iso3}.png" for instance. We havesave_image_az()
called to handle that.output_location
islocal
, then we also expectoutput_dir
as above, and create asave_image_local()
function for this.In this way, we can default to saving to Mailchimp, but have capacity to save to Azure as default in our
test/manual/maps
directory for instance, but also able to save locally for iterative development if we are interested.