Design vignette #69

Degoot-AM · 2023-12-19T11:18:19Z

No description provided.

vignettes/design_principle.Rmd

Bisaloo · 2023-12-21T10:06:27Z

vignettes/design_principle.Rmd

+
+1. The `read_from_api()` function takes only two arguments to import data from an API: an object of class **credential** and an object of class **api**.
+
+The **credential** object is a list-like object that stores the information required to access the API. Its main attributes are:


Did you consider the benefits & downsides of list vs named vector?

Or should it be an actual class?

Is there value of pulling the credential object outside of the read_from_() call?

The workflow would be:

authenticate(name, credentials) read() read() read()

I opt for the class option. And yes, we can add more auxiliary functions for authentication--as you mentioned--, verification, and for checking the accessibility to API/database. Moreover, such steps are done internally when to you call the read_from_* functions, but we can modularize them as separate functions.

The idea of what I'm proposing is that users would not have to repeat the same arguments over and over if they are trying to get different pieces of data from the same source with successive read_from_() calls.

We had this discussion internally here and we decided to make the 2 implementations and compare them.

My only reserves about this that, in read_from_*, we will have a long list of arguments (including both username, password, url, database specific arguments and api-specific arguments).

The arguments were made in previous PRs that the list of arguments are to long and we need to act on this.

Another option will consist in making the credential class such that it returns a connexion object as described below:

It will have an authenticate() method that returns a connexion object. This method will be called internally by the class constructor.

It's print() method will display the user's credential details and a string to tell whether the connexion was successfully established or not.

That way, the credential object (can be called authentication object!!!) is created once and will be used in any call of the read_from_* function, until the connection expires.

Co-authored-by: Hugo Gruson <Bisaloo@users.noreply.github.com>

add {xml2} as dependency

@nord

* add the examples to the functions documentations * build documentation for show_tables() and visualise_table() * Update CITATION.cff * update function's examples * fix linters * add new lines * remove unnecessary nolint * use explicit URL * update show_example_file documentation * align argument in function documentation * Automatic readme update * add examples and remove dontrun{} when not needed in DHIS2 related functions * add examples and remove dontrun{} when not needed in Fingertips related functions * add examples and remove dontrun{} when not needed in REDCap related functions * add examples and remove dontrun{} when not needed in server related functions * add examples and remove dontrun{} when not needed in readepi related functions * run devtools::document() * remove README.html * rearrange arguments in read_from_readcap example * complete test example for dhis2_get_attributes_from_user * Update _pkgdown.yml to account for read_from_fingertips * update on dhis2_get_attributes_from_user() * Update CITATION.cff * remove examples in internal functions documentation - for functions without dontrun{} * Update R/read_from_fingertips.R remove @nord in the function documentation Co-authored-by: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> --------- Co-authored-by: Karim-Mane <karimanee@outlook.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Karim MANE <84502011+Karim-Mane@users.noreply.github.com> Co-authored-by: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com>

* replace {httr} by {httr2} * Update CITATION.cff * fix linter and run devtools::document() * update test file for check_dhis2_attributes-helpers.R * use {httr2} to submit the query parameters * fix linters * allow for the query parameters to be specified as a list by the user * update the test files to account for the fact that the query parameters are provided as a list by the user * fix linters * use a different areaTypeId in the examples and tests * update the mock files * update test for readepi() * update test-readepi.R * fix issue with reading from DHIS2 * Automatic readme update * comment out the mock testing in Fingertips test files * use "%>%" instead of "|>" for harmonisation. * update the URL to the play DHIS2 instance --------- Co-authored-by: GitHub Action <action@github.com>

vignettes/design_principle.Rmd

Bisaloo

Different versions of the same figure are saved under different names. Please review and keep only the latest version.

vignettes/design_principle.Rmd

Bisaloo · 2024-05-28T07:29:57Z

vignettes/design_principle.Rmd

+The `read()` functions return a `list` object containing one or more `data frames`. Each `data frame` corresponds to the data from a specified source.
+The `get_metadata()` function returns a data dictionary containing information about the data structure.
+The `authenticate()` function returns a connection object that is used in the query request.


Am I misunderstanding something or does this not match what the diagram is describing (read_from_api(), read_from_server(), authenticate())?

There are few changes in the design diagram. We now want to split read_from_api() into small functions that are specific to a given HIS. This, we think, will make it easy to perform targetted updates on the way data is imported from a HIS, when there is any changes from that API.

I have updated the design diagram in commit 41f41e8.

Bisaloo · 2024-05-28T13:06:11Z

DESCRIPTION

@@ -34,8 +36,10 @@ Imports:
    odbc,
    pool,
    REDCapR,
-    RMySQL
+    RMySQL,
+    xml2


Was this dependency added by mistake?

No - {RMySQL} is the {DBI} driver used when connecting to a MySQL database.

{xml2} is dependency for importing data from Fingertips or DHIS2. It was required for some examples to pass the check.

Yes, but it's unrelated to this PR. Maybe a conflict fix that went wrong? Anyways, as long as you don't have conflicts, let's go with it.

That is correct. There were some conflicts. I realised that some examples needed the {xml2} package when running devtools::check().
The {RMySQL} were (or should have been) there already as the example related to fetching data from RDBMS is using a MySQL database.

Bisaloo reviewed Dec 21, 2023

View reviewed changes

Degoot-AM self-assigned this Dec 21, 2023

Karim-Mane and others added 17 commits May 22, 2024 13:13

add package design vignette and readepi roadmap image

5bc8524

add links to HIS of interest

175ed9d

Update design_principle.Rmd

906a39f

add link to Epiverse website

d080695

complete the package design vignette

3e60346

adjust size of design diagram

4db23f5

delete previous design diagrams

90913d9

use comma to separate depencencies in package design vignette

1d99583

Update vignettes/design_principle.Rmd

f24ec8e

Co-authored-by: Hugo Gruson <Bisaloo@users.noreply.github.com>

add commits from Hugo

3a74903

update documentation about get_metadata() and add dependencies

667dc65

update the design vignette

424b867

Update DESCRIPTION

3c1e87c

add {xml2} as dependency

Update CITATION.cff

7b1fb7a

Update CITATION.cff

8a4b82d

Karim-Mane force-pushed the design_vignette branch from 1619a6d to 8a4b82d Compare May 22, 2024 13:24

Karim-Mane and others added 3 commits May 22, 2024 14:05

update the documentation on the authenticate() function

e997621

Update the diagrams and function documentation

064d80e

Update CITATION.cff

fdfc6a0

Karim-Mane requested a review from Bisaloo May 23, 2024 23:32

Add cph and fnd info to the DESCRIPTION file

c7d8f1b

Bisaloo reviewed May 28, 2024

View reviewed changes

vignettes/design_principle.Rmd Outdated Show resolved Hide resolved

Bisaloo reviewed May 28, 2024

View reviewed changes

fix typos and update design diagram

41f41e8

Bisaloo reviewed May 28, 2024

View reviewed changes

Karim-Mane added the documentation Improvements or additions to documentation label May 28, 2024

update design vignette: add strategies to minimize API security risks

e457673

Karim-Mane requested a review from Bisaloo May 30, 2024 13:00

Karim-Mane self-requested a review June 10, 2024 11:50

Karim-Mane approved these changes Jun 10, 2024

View reviewed changes

Karim-Mane merged commit 55c8eb6 into main Jun 10, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design vignette #69

Design vignette #69

Degoot-AM commented Dec 19, 2023

Bisaloo Dec 21, 2023

Bisaloo Dec 21, 2023

Degoot-AM Dec 21, 2023

Bisaloo Jan 9, 2024

Karim-Mane Feb 15, 2024

Karim-Mane Feb 16, 2024

Bisaloo left a comment

Bisaloo May 28, 2024

Karim-Mane May 28, 2024

Bisaloo May 28, 2024

Karim-Mane May 28, 2024

Bisaloo May 28, 2024

Karim-Mane May 28, 2024


		1. The `read_from_api()` function takes only two arguments to import data from an API: an object of class credential and an object of class api.

		The credential object is a list-like object that stores the information required to access the API. Its main attributes are:

Design vignette #69

Design vignette #69

Conversation

Degoot-AM commented Dec 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bisaloo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment