Tefera update branch #100

Tefera19 · 2024-03-14T18:18:29Z

I have added `detail' argument in the main functions.

Tefera19 · 2024-03-28T14:59:08Z

I have updated country_list (cl_validation_raw()) script, I have also added a function that clean validation report object from the environment variable.

…ever it is required

Tefera19 · 2024-04-10T13:54:24Z

Hi @randrescastaneda ,

I have updated test-countries-validation() as per the update implemented in pip_countries(). I have also added gls in the unit test functions when it is requred.

randrescastaneda · 2024-04-10T18:00:55Z

Thank you so much. I'll check and get back to you.

Tefera19 · 2024-04-15T14:21:47Z

Hi @randrescastaneda , Thank you for the feedback, I have updated the unit test functions. But, there are two outstanding issues that I couldn't resolve ("test-load_aux.R" and "test-pip_sign_save.R").

randrescastaneda · 2024-04-22T22:17:33Z

DESCRIPTION

@@ -53,7 +53,12 @@ Imports:
    joyn,
    dm,
    config,
-    collapse
+    collapse,


Do we need all these dependencies? For example, do we need covr? Could it be moved to suggests?

I moved covr package to suggests section.

randrescastaneda · 2024-04-22T22:20:16Z

NAMESPACE

@@ -2,8 +2,26 @@

 export("%>%")
 export(auto_aux_update)
+export(cl_validate_raw)


Is there a reason to export all the validation functions? I think they are meant to be used internally. If they are, you should use #' @keywords internal in the roxygen chunk of those functions. If they are not, please provide a short explanation. Thanks !

As you have suggested, I included #' @Keywords internal to roxygen chunk to all the data validation functions.

Thanks

randrescastaneda · 2024-04-22T22:22:40Z

R/cl_validate_raw.R

+  validate(cl, name = "CL raw data validation") |>
+    validate_if(is.character(country_code),
+                description = "`country_code` should be character") |>
+    validate_cols(in_set(c("ABW", "AFG", "AGO", "ALB", "AND", "ARE", "ARG", "ARM", "ASM", "ATG", "AUS", "AUT", "AZE",


This should not be hardcoded. The list of countries could vary from one year to another (though unlikely). Please use unique() over the column with the country codes in the priceframework data (pfw)

Thank you for the comment, I pulled the country list from pfw data.

Thanks

randrescastaneda · 2024-04-22T22:26:51Z

R/cpi_validate_raw.R

+#' @importFrom assertr in_set not_na is_uniq
+#'
+#' @export
+cpi_validate_raw <- function(cpi, detail = getOption("pipaux.detail.raw")){


I sent you an email showing you a real case of a problem that this check should find out... please add a check for that. THANKS!

The case you have sent me via email shows same cpi value for multiple years. What would be the minimum years where the cpi value could be same? In the example, cpi value are same for four survey reference years. In the latest PR, I checked same cpi / duplicate cpi.

Thanks

randrescastaneda · 2024-04-22T22:30:32Z

R/get_error_validation.R

+
+    }
+
+    cli::cli_inform("Validation report ('validation_report') has been added to the environment varaible (.pipaux).")


Please use cli syntax. like "({.field validation_report})" . See here: https://cli.r-lib.org/reference/inline-markup.html

Thank you, I implemented your suggestion.

Thanks

randrescastaneda · 2024-04-22T22:35:19Z

R/pip_country_list.R

@@ -27,6 +28,9 @@ pip_country_list <- function(action = c("update", "load"),
    ## Special national accounts --------
    cl <- pip_country_list_update(class_branch = class_branch)

+  # validate country list raw data
+    cl_validate_raw(cl, detail = detail)


I think i am a little confused. Where does cl_validate_output() go? Thanks.

The results from this function depends on value of detail argument. By default (detail = FALSE), for example result cl_validate_output() goes to the console. If detail = TRUE, the result will be emailed (now - will be printed to the console when we run send_report() function)

Thanks

randrescastaneda · 2024-04-22T22:39:12Z

R/send_report.R

+
+    print(.pipaux$validation_report)
+
+  #   fname <- file.path(tempdir(), "data_validation_report.csv")


We will work on this later, right?

Yes, it will work !

randrescastaneda · 2024-04-22T22:41:26Z

R/weo_validate_raw.R

+                description = "`2026` should be character") |>
+    validate_if(is.character(`2027`),
+                description = "`2027` should be character") |>
+    validate_if(is.character(`2028`),


There should be an easier way to check all these columns. First, these columns will increase and we can't update the code each time they do. Second, the importance here is that they years exists, so that's what is needed to be checked. There should no be jumps in the years.

Thanks for the comment, I have implemented your comment in the latest PR.

randrescastaneda · 2024-04-22T22:43:07Z

Data/git_metadata.csv

@@ -0,0 +1,25 @@
+Repo,hash,branch


I am not sure, this file should be part of the PR. It could break something else later on. For now, remove it from the PR. Thanks.

I have removed this file from the latest PR.

randrescastaneda

Hi @Tefera19 ,

I think the PR is really, really good. Great job. Thanks.

I added a few comments. Please address them and push to the same branch. You don't neet to create a new PR. Just let me know when it is ready to review again. Thanks!

randrescastaneda · 2024-10-07T15:37:49Z

Hi @Tefera19 ,

I'm closing this PR because we need this changes to be added to branch DEV_v2_tefera_testing.

Thank you.

Tefera19 added 14 commits January 29, 2024 16:06

CPI raw and output sample data validation scripts

31c6afc

CPI raw and output datasets validation scripts

9ec8dea

Update cpi raw and output data validation scrips

82ec25c

Add validation scripts

3295449

zzz.R updated

fd3693c

Updated validation scripts

a439e4d

DESCRIPTION and NEMSPACE updated

7ad3052

Adding validation rule for metadata file

7fa88c2

Adding validation rule for npl and income group files

e0aefac

Add 'detail' argument in higher level functions

3edaa70

Add 'detail' argument in higher level functions

c8c6989

Unit test functions are added

38719e3

Merge branch 'tefera_dev' into tefera_update_branch

0056f16

Update validation scripts

29eb56e

Tefera19 added 3 commits April 9, 2024 10:57

Merge branch 'tefera_dev' into tefera_update_branch

4dba4f9

check the correct file

eb0d961

update countries validate function and gls to unit test function wher…

e3bdc25

…ever it is required

Updated unit test functions

0a790be

randrescastaneda reviewed Apr 22, 2024

View reviewed changes

randrescastaneda requested changes Apr 22, 2024

View reviewed changes

Tefera19 added 7 commits May 7, 2024 15:35

Update based on comments

bca92ed

Validation functions are updated

a5438ea

Change function name from merge_aux_files to merger_aux

31b2909

pip_weo_clean() has been updated

90f7b44

Temporary folder loacation has been updated

c7dd7ae

test file for merger_aux() added

0a4f936

Validating, test, and pip functions are updated

add9f52

randrescastaneda closed this Oct 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tefera update branch #100

Tefera update branch #100

Tefera19 commented Mar 14, 2024

Tefera19 commented Mar 28, 2024

Tefera19 commented Apr 10, 2024

randrescastaneda commented Apr 10, 2024

Tefera19 commented Apr 15, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda Apr 22, 2024

Tefera19 May 8, 2024

randrescastaneda left a comment

randrescastaneda commented Oct 7, 2024


		}

		cli::cli_inform("Validation report ('validation_report') has been added to the environment varaible (.pipaux).")


		print(.pipaux$validation_report)

		# fname <- file.path(tempdir(), "data_validation_report.csv")

Tefera update branch #100

Tefera update branch #100

Conversation

Tefera19 commented Mar 14, 2024

Tefera19 commented Mar 28, 2024

Tefera19 commented Apr 10, 2024

randrescastaneda commented Apr 10, 2024

Tefera19 commented Apr 15, 2024

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

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

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

randrescastaneda left a comment

Choose a reason for hiding this comment

randrescastaneda commented Oct 7, 2024