search

PIP-Technical-Team / pipapi

What the Package Does (One Line, Title Case)
https://pip-technical-team.github.io/pipapi/
Other
3 stars 0 forks source link

Vectorize povline #403

Open shahronak47 opened 3 months ago

shahronak47 commented 3 months ago

Hi Andres,

This is the first draft of vectorize povline. I just want to confirm the expected output. To test this install vectorize-povline branch in wbpip using - 

metapip::install_branch("wbpip", "vectorize-povline")

You can then test - 

devtools::load_all()

out1 <- pip(country = "AGO",
     year = 2000,
     povline = 1.9,
     lkup = lkups$versions_paths$`20220810_2017_01_02_TEST`)

out2 <- pip(country = "AGO",
            year = 2000,
            povline = 1.675,
            lkup = lkups$versions_paths$`20220810_2017_01_02_TEST`)         

out3 <- pip(country = "AGO",
            year = 2000,
            povline = c(1.675, 1.9),
            lkup = lkups$versions_paths$`20220810_2017_01_02_TEST`)

identical(rbind(out2, out1), out3)
randrescastaneda commented 3 months ago

Hi @shahronak47 ,

I tested the output and it is looking very nice. Yes, this is what I had in mind. Thank you.

So, These would be the next steps

  1. Vectorize properly prod_md_compute_pip_stats() and prod_gd_compute_pip_stats() in prod_compute_pip_stats to avoid the lapply() function.
  2. make sure it is also implemented in fillgaps, (fg_pip())
  3. Make sure it is implemented in aggregate data, in (pip_grp_logic)
  4. Add tests.
shahronak47 commented 2 weeks ago

Hi @randrescastaneda , I have pushed the fixes and now all the test cases from https://github.com/PIP-Technical-Team/pipapi/pull/413 pass. You can do some more round of testing if you have time or else we will wait till mid-September before merging this PR. Thank you for providing with the test file, it was very helpful.