Open Bisaloo opened 1 year ago
list(data, metadata)
. use metadata = NA
if not applicabledo.call()
and dplyr::bind_rows()
, paste()
and glue::glue_collapse()
Thanks for getting started on this. Please make each of these points a separate PR.
I have already made the changes for setting argument's default values
and argument consistency
.
I will make the pull request for these 2 now.
The goal of readepi is to provide a unified interface to various health information system or common epidemiological databases. A key principle and the challenge is the harmonization of these very different data sources.
This harmony should also visible internally in low-level parsers to 1. facilitate the understanding of the package structure, thus making easier to maintain it and detect potential issues, 2. possibly eventually export these low-level parsers, as discussed in #45.
I see the following sources of inconsistency that should be addressed:
login()
ormake_api_request()
when they only apply to a specific data source. This video from Felienne Hermans, a researcher in software engineering, is a good resource of this topic.https://github.com/epiverse-trace/readepi/blob/11aa71e6618946527fd517ba238c8830fe6a8795/R/read_from_dhis2.R#L39-L41
https://github.com/epiverse-trace/readepi/blob/11aa71e6618946527fd517ba238c8830fe6a8795/R/read_from_dhis2-helpers.R#L13
assert_credentials(username, password)
, orassert_url()
)list(data)
where others returnlist(data, metadata)
. I understand metadata may be available only in some cases but the return values should be consistent across functions, even if it means returninglist(data = data, metadata = NA)
. This can again be enforced via roxygen's documentation inheritance.do.call(rbind.data.frame, ...)
vsdplyr::bind_rows(...)
;paste(collapse)
vsglue::glue_collapse()
, passing request parameters tohttr::GET()
as a list or directly in the URL, etc.)