Conversation
Tested in "SNT Testing" - COD
irautu-bluesquare
left a comment
There was a problem hiding this comment.
Nice work, mentioned some minor changes, most of them can be added for a future rework if you don't have time now
| " country_code = COUNTRY_CODE, \n", | ||
| " suffix = \"_dhis2_raw_analytics.parquet\" \n", | ||
| " )\n", | ||
| "printdim(routine_data)" |
There was a problem hiding this comment.
Not crucial, but this might be a bit surprising/difficult to understand for someone who's less technical ("data dimensions" is jargon in our field). See my comment in the function, for more details.
|
|
||
| # To move to ./code/snt_utils.r ... (see https://bluesquare.atlassian.net/browse/SNT25-591 ) | ||
| printdim <- function(df, name = deparse(substitute(df))) { | ||
| cat("Dimensions of", name, ":", nrow(df), "rows x", ncol(df), "columns\n\n") |
There was a problem hiding this comment.
What is the "x" in "rows x"? Also, not crucial, but the term "data dimensions" isn't widely understood and it's redundant here, since you already mention 'rows' and 'columns' which is clearer.
| "# caption = \"Liste des éléments de données extraits, classés par indicateur\",\n", | ||
| "# col.names = c(\"ID de l'élément\", \"Nom de l'élément de donnée\", \"Indicateur\")\n", | ||
| "# )\n", | ||
| "## GP: modified to match in style the table of next cell\n", |
There was a problem hiding this comment.
yes, i think this is going to be clearer!
There was a problem hiding this comment.
Very nice!
I think, both to keep in line with our approach in the reports and with best practices for testing in general, that the code and plots should become functions. Not sure if you have the time to do it now, so if you don't, you can disregard this and we'll add it for a future rework.
There was a problem hiding this comment.
To go back to our discussions in and after a previous meeting, about rendering: so far the best method I've found when rendering a plot, to make sure it keeps the size I want, is to:
- import IRdisplay
- create the ggplot_object under a variable name (not displaying it)
- set a variable with a dynamic width/height (optional, only if necessary and if yes, preferably set only one of the dimensions)
- create the path_for_saving
- save the ggplot_object in its path_for_saving (with the set width/height), removing the warning about the size, which is confusing to the user. for this, i use "suppressMessages(ggsave(filename = path_for_saving, plot = ggplot_object, ...))"
- call the plot from path_for_saving with display_png(file = path_for_saving)
This ensures the plots stay the size you want and that they're properly rendered in the report
| } | ||
|
|
||
| make_point_geojson <- function(lat, lon) { | ||
| sprintf('{"type": "Point", "coordinates": [%f, %f]}', lon, lat) |
There was a problem hiding this comment.
maybe correct the order of the parameters, because in the function calling, it's first latitude and then longitude
SNT25-488 --- Reporting nb + r utils of reporting of "A.1 DHIS2 Extract"
https://bluesquare.atlassian.net/browse/SNT25-488
Done
kable()style (it was confusing to have 2 rendering styles, and kable outputs are truncated when too long)Yet to be addressed (not great but not breaking):
### 6.:🤌🏼 # GP (2026-08-06): why do we get "white" (or blank) tiles? Missing data should get a 0% ...routine_databecause at step### 5.it is transformed and overwritten, so if used as is it breaks at point### 7.(hence need to keep the second import from dataset). See comment:⚠️ # GP: need to keep this import and transformation even if repeated from upstream) ...Tested in:
☑️ Also checked: it renders in Orchestrator WebApp html embedded preview
⛙ Important: always merge branch with
Rebaseso changes are replayed on top (avoid re-surfacing of older code)