Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rdhs: submission for Ropensci #238

Closed
11 of 19 tasks
OJWatson opened this issue Jul 20, 2018 · 63 comments
Closed
11 of 19 tasks

rdhs: submission for Ropensci #238

OJWatson opened this issue Jul 20, 2018 · 63 comments

Comments

@OJWatson
Copy link

Summary

  • What does this package do? (explain in 50 words or less):

It interacts with the Demographic and Health Survey (DHS) Program API (https://api.dhsprogram.com), and provides tools to use the API to ease identifying, downloading, loading and analysing the raw survey data collected by the DHS.

  • Paste the full DESCRIPTION file inside a code block below:
Package: rdhs
Type: Package
Title: API Client and Dataset Management for the Demographic and Health Survey (DHS) Data
Version: 0.5.0
Authors@R: c(
  person("OJ", "Watson", role=c("aut", "cre"),
         email="o.watson15@imperial.ac.uk"),
  person("Jeff", "Eaton", role="aut"))
Maintainer: OJ Watson <o.watson15@imperial.ac.uk>
URL: https://ojwatson.github.io/rdhs/
BugReports: https://github.com/OJWatson/rdhs/issues
Description: Provides a client for (1) querying the DHS API for survey indicators
  and metadata (https://api.dhsprogram.com/#/index.html), (2) identifying surveys
  and datasets for analysis, (3) downloading survey datasets from the DHS website,
  (4) loading datasets and associate metadata into R, and (5) extracting variables
  and combining datasets for pooled analysis.
LazyData: TRUE
Depends: R (>= 3.3.0)
Imports: 
    R6,
    httr,
    jsonlite,
    foreign,
    magrittr,
    rappdirs,
    digest,
    storr,
    xml2,
    qdapRegex,
    rgdal,
    getPass,
    haven,
    iotools
Suggests:
    testthat,
    knitr,
    rmarkdown,
    ggplot2,
    survey,
    data.table,
    microbenchmark
License: MIT + file LICENSE
RoxygenNote: 6.0.1
VignetteBuilder: knitr
Language: en-GB

  • URL for the package (the development repository, not a stylized html page):

https://github.com/OJWatson/rdhs

  • Please indicate which category or categories from our package fit policies this package falls under *and why(? (e.g., data retrieval, reproducibility. If you are unsure, we suggest you make a pre-submission inquiry.):

    • data access, because it interacts with the DHS API
    • data retrieval, because it helps download and load the raw datasets from the DHS website, after using the API to first identify those survey datasets relevant for your analysis
  •   Who is the target audience and what are scientific applications of this package?  

Global Health Researchers and Policy makers. The DHS data has been used in well over 20,000 academic studies (based on google scholar search for "DHS" AND "demographic and health survey") that have helped shape progress towards targets such as the Sustainable Development Goals and inform health policy such as detailing trends in child mortality and characterising the distribution and use of insecticide-treated bed nets in Africa. The package will help assist researchers who use R for these purposes rather than/don't have access to stata/sas (these datasets are the published datasets by the DHS program), as well as serve to simplify commonly required analytical pipelines. The end result aims to increase the end user accessibility to the raw data and create a tool that supports reproducible global health research.

There are a number of other R pacakges that work with DHS data in various ways. A quick search of github for "DHS" and R shows 39 repos, however the majority are small custom scripts.

1 repo looks just at interacting with the DHS API, but it hasn't been added to for almost a year, and the API endpoint functions do not cover all the endpoints available nor allow you to query each endpoint by all the possible query terms. It also requires the user to know query terms rather than having them as arguments.

1 repo also looks at downloading the survey datasets from the website (and it was used initially when designing these fucntions with rdhs). However, it skips over large dataset files, has some bugs depending on the character length of your login credentials, and does not allow you to read in all the datasets available from the website. [ FYI: we don't read in .sas7bdat (we are writing a parser for the oddly formed catalog files provided by the DHS website for these) or hierarchal dataset files as we have a parser for the flat equivalent of hierarchal dataset. In theory each file format should be the same data, so having one parser that works is sufficient, but we have found that the flat and spss data formats have the most complete meta data for the data variable labels).

There are then a few repos that do bespoke pieces of analysis (2 of which are on CRAN) looking at spatial analysis and calculating survey statistics. We are hoping to bring these onboard, either by wrapping them to use the output of our downloaded harmonised datasets, or by writing additional tools for downstream analysis (see TODO.md).

  •   If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Requirements

Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.
  • has a CRAN and OSI accepted license.
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a vignette with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration, including reporting of test coverage, using services such as Travis CI, Coveralls and/or CodeCov.
  • I agree to abide by ROpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

Publication options

  • Do you intend for this package to go on CRAN?
  • Do you wish to automatically submit to the Journal of Open Source Software? If so:
    • The package has an obvious research application according to JOSS's definition.
    • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
    • The package is deposited in a long-term repository with the DOI:
    • (Do not submit your package separately to JOSS)
  • Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
    • The package is novel and will be of interest to the broad readership of the journal.
    • The manuscript describing the package is no longer than 3000 words.
    • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
    • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no gaurantee that your manuscript willl be within MEE scope.)
    • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
    • (Please do not submit your package separately to Methods in Ecology and Evolution)

Detail

  • Does R CMD check (or devtools::check()) succeed? Paste and describe any errors or warnings:

Yes:

R CMD check results
0 errors | 0 warnings | 0 notes
  • Does the package conform to rOpenSci packaging guidelines? Please describe any exceptions:

  • If this is a resubmission following rejection, please explain the change in circumstances:

  • If possible, please provide recommendations of reviewers - those with experience with similar packages and/or likely users of your package - and their GitHub user names:

@annakrystalli
Copy link
Contributor

annakrystalli commented Jul 26, 2018

Editor checks:

  • Fit: The package meets criteria for fit and overlap
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  • License: The package has a CRAN or OSI accepted license
  • Repository: The repository link resolves correctly
  • Archive (JOSS only, may be post-review): The repository DOI resolves correctly
  • Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

Editor comments

Hello @OJWatson 👋 and many thanks for your submission!

So the package passes most of the initial checks apart from the fact the testing suite is failing for me on my machine. Any ideas?

I've included relevant outputs of checks and tests below. The main error seems to refer to an unused argument (TRUE), I think by tempdir(TRUE) calls?

Otherwise, there's only a minor goodpractice suggestion flag about long lines in a couple of places.

If you could help with debugging the testing failure, I can start looking for reviewers?

checks

pkg_dir <- "/Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs"
devtools::check(pkg_dir)

#> R CMD check results
#> 1 error  | 0 warnings | 1 note
#> checking tests ... ERROR
#>   Running ‘testthat.R’
#> Running the tests in ‘tests/testthat.R’ failed.
#> Last 13 lines of output:
#>   ══ testthat results  ════════════════════════════════════════════════════
#>   OK: 35 SKIPPED: 21 FAILED: 19
#>   1. Error: geojson works (@test_api_endpoints.R#60) 
#>   2. Error: dhs_countries works (@test_api_endpoints.R#89) 
#>   3. Error: dhs_data works (@test_api_endpoints.R#106) 
#>   4. Error: dhs_dataUpdates works (@test_api_endpoints.R#132) 
#>   5. Error: dhs_datasets works (@test_api_endpoints.R#141) 
#>   6. Error: dhs_indicators works (@test_api_endpoints.R#154) 
#>   7. Error: dhs_info works (@test_api_endpoints.R#174) 
#>   8. Error: dhs_publications works (@test_api_endpoints.R#183) 
#>   9. Error: dhs_survey_characteristics works (@test_api_endpoints.R#203) 
#>   1. ...
#>   
#>   Error: testthat unit tests failed
#>   Execution halted
#> 
#> checking R code for possible problems ... NOTE
#> find_rdhs_config: possible error in tempdir(TRUE): unused argument
#>   (TRUE)
#> set_rdhs_config: possible error in tempdir(TRUE): unused argument
#>   (TRUE)

Created on 2018-07-26 by the reprex package (v0.2.0).

tests

pkg_dir <- "/Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs"
devtools::test(pkg_dir)

#> Loading rdhs
#> Loading required package: testthat
#> Testing rdhs
#> Warning: `encoding` is deprecated; all files now assumed to be UTF-8
#> ✔ | OK F W S | Context
#> |  0       | API calls|  0     1 | API calls|  0     1 | API calls
#> ─────────────────────────────────────────────────────────────────────────
#> test_api_calls.R:4: skip: can request api through dhs_api_request via client
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | API endpoints|  0     1 | API endpoints|  0 1   1 | API endpoints|  0 2   1 | API endpoints|  0 3   1 | API endpoints|  0 4   1 | API endpoints|  0 5   1 | API endpoints|  0 6   1 | API endpoints|  0 7   1 | API endpoints|  0 8   1 | API endpoints|  0 9   1 | API endpoints|  0 10   1 | API endpoints|  0 11   1 | API endpoints|  0 12   1 | API endpoints|  0 12   1 | API endpoints
#> ─────────────────────────────────────────────────────────────────────────
#> test_api_endpoints.R:8: skip: format catches and al_results tests
#> No authentication available
#> 
#> test_api_endpoints.R:60: error: geojson works
#> unused argument (TRUE)
#> 1: dhs_data(countryIds = "SN", surveyYearStart = 2014, breakdown = "subnational", 
#>        returnGeometry = TRUE, f = "geojson") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:60
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:135
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:89: error: dhs_countries works
#> unused argument (TRUE)
#> 1: dhs_countries(countryIds = "SN", surveyYearStart = "2010", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:89
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:440
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:106: error: dhs_data works
#> unused argument (TRUE)
#> 1: dhs_data(countryIds = "EG", indicatorIds = "FE_FRTR_W_TFR", selectSurveys = "latest", 
#>        all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:106
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:135
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:132: error: dhs_dataUpdates works
#> unused argument (TRUE)
#> 1: dhs_data_updates(lastUpdate = "20150901", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:132
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:1057
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:141: error: dhs_datasets works
#> unused argument (TRUE)
#> 1: dhs_datasets(countryIds = "EG", selectSurveys = "latest", surveyYearStart = 2000, 
#>        surveyYearEnd = 2016, surveyType = "DHS", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:141
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:837
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:154: error: dhs_indicators works
#> unused argument (TRUE)
#> 1: dhs_indicators(countryIds = "EG", all_results = FALSE, indicatorIds = "FE_FRTR_W_TFR") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:154
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:230
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:174: error: dhs_info works
#> unused argument (TRUE)
#> 1: dhs_info(infoType = "version", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:174
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:345
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:183: error: dhs_publications works
#> unused argument (TRUE)
#> 1: dhs_publications(countryIds = "EG", all_results = FALSE, selectSurveys = "latest") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:183
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:740
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:203: error: dhs_survey_characteristics works
#> unused argument (TRUE)
#> 1: dhs_survey_characteristics(countryIds = "EG", surveyYearStart = 2000, surveyYearEnd = 2016, 
#>        surveyType = "DHS", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:203
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:638
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:219: error: dhs_surveys works
#> unused argument (TRUE)
#> 1: dhs_surveys(countryIds = "EG", surveyYearStart = 2000, surveyYearEnd = 2016, 
#>        surveyType = "DHS", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:219
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:552
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:233: error: dhs_tags works
#> unused argument (TRUE)
#> 1: dhs_tags(indicatorIds = "FE_FRTR_W_TFR", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:233
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:1002
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> 
#> test_api_endpoints.R:242: error: dhs_uiUpdates works
#> unused argument (TRUE)
#> 1: dhs_ui_updates(lastUpdate = "20150901", all_results = FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:242
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:285
#> 3: check_for_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:27
#> 4: rdhs_setup() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:7
#> 5: find_rdhs_config() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/zzz.R:27
#> 6: file.path(tempdir(TRUE), "rdhs/rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:7
#> ─────────────────────────────────────────────────────────────────────────
#> 
#> ══ Terminating early ════════════════════════════════════════════════════
#> Too many failures

Created on 2018-07-26 by the reprex package (v0.2.0).

goodpractice

Checks for r package development best practice

pkg_dir <- "/Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs"
goodpractice::gp(pkg_dir)

#> Preparing: covr
#> Preparing: cyclocomp
#> Installing 5 packages: fansi, openssl, pillar, sp, utf8
#> Preparing: description
#> Preparing: lintr
#> Preparing: namespace
#> Preparing: rcmdcheck
#> ── GP rdhs ────────────────────────────────────────────────────────────────
#> 
#> It is good practice to
#> 
#>   ✖ avoid long code lines, it is bad for readability. Also, many
#>     people prefer editor windows that are about 80 characters
#>     wide. Try make your lines shorter than 80 characters
#> 
#>     R/config.R:282:1
#>     R/read_dhs_flat.R:113:1
#> 
#> ───────────────────────────────────────────────────────────────────────────

Created on 2018-07-26 by the reprex package (v0.2.0).

@OJWatson
Copy link
Author

Hi @annakrystalli - thank you for checking this through.

  • Have just had a look and I didn't notice that tempdir(check) was only introduced in R 3.5, so I've just pushed some changes to catch for the R version before using this argument for backward compatibility.
  • Have also trimmed those lines now.

Also a lot of the tests skip as you need the login details for the dummy account I've set up with the DHS website. These are stored within rdhs.json.tar.enc and are unlocked with a key that I've stored within my travis account. I can send the key if it would be helpful, or if there is a better way to share these securely then let me know as I'm still quite new to this sort of stuff.

Thank you again,

OJ

@annakrystalli
Copy link
Contributor

Thanks for the quick response @OJWatson!

So I re-cloned the repo but I'm still getting errors during the tests. I include the output below, with some questions at the end of the comment:

check

pkg_dir <- "/Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs"
devtools::check(pkg_dir)

#> R CMD check results
#> 1 error  | 0 warnings | 1 note
#> checking tests ... ERROR
#>   Running ‘testthat.R’ [16s/486s]
#> Running the tests in ‘tests/testthat.R’ failed.
#> Last 13 lines of output:
#>   downloaded 80 KB
#>   
#>   ══ testthat results  ════════════════════════════════════════════════════
#>   OK: 64 SKIPPED: 21 FAILED: 8
#>   1. Error: geojson works (@test_api_endpoints.R#60) 
#>   2. Error: ETAR71FL.ZIP test (@test_downloads.R#139) 
#>   3. Error: ugir41fl.zip test (@test_downloads.R#148) 
#>   4. Error: zip file ending test (@test_downloads.R#157) 
#>   5. Error: Hierarchal and sas7bdat dataset test (@test_downloads.R#168) 
#>   6. Error: Large India Files (@test_downloads.R#214) 
#>   7. Error: Odd nesting India (@test_downloads.R#223) 
#>   8. Error: surveyId in extract (@test_extraction.R#109) 
#>   
#>   Error: testthat unit tests failed
#>   Execution halted
#> 
#> checking R code for possible problems ... NOTE
#> tempdir_check: possible error in tempdir(TRUE): unused argument (TRUE)

Created on 2018-07-26 by the reprex package (v0.2.0).

test

pkg_dir <- "/Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs"
devtools::test(pkg_dir)
#> Loading rdhs
#> Loading required package: testthat
#> Testing rdhs
#> Warning: `encoding` is deprecated; all files now assumed to be UTF-8
#> ✔ | OK F W S | Context
#> |  0       | API calls|  0     1 | API calls|  0     1 | API calls
#> ─────────────────────────────────────────────────────────────────────────
#> test_api_calls.R:4: skip: can request api through dhs_api_request via client
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | API endpoints|  0     1 | API endpoints|  0 1   1 | API endpoints|  1 1   1 | API endpoints|  2 1   1 | API endpoints|  3 1   1 | API endpoints|  4 1   1 | API endpoints|  5 1   1 | API endpoints|  6 1   1 | API endpoints|  7 1   1 | API endpoints|  8 1   1 | API endpoints|  9 1   1 | API endpoints| 10 1   1 | API endpoints| 11 1   1 | API endpoints| 12 1   1 | API endpoints| 13 1   1 | API endpoints| 14 1   1 | API endpoints| 15 1   1 | API endpoints| 16 1   1 | API endpoints| 17 1   1 | API endpoints| 18 1   1 | API endpoints| 19 1   1 | API endpoints| 20 1   1 | API endpoints| 21 1   1 | API endpoints| 22 1   1 | API endpoints| 23 1   1 | API endpoints| 24 1   1 | API endpoints| 25 1   1 | API endpoints| 26 1   1 | API endpoints| 27 1   1 | API endpoints| 28 1   1 | API endpoints| 29 1   1 | API endpoints| 29 1   1 | API endpoints [303.2 s]
#> ─────────────────────────────────────────────────────────────────────────
#> test_api_endpoints.R:8: skip: format catches and al_results tests
#> No authentication available
#> 
#> test_api_endpoints.R:60: error: geojson works
#> argument is of length zero
#> 1: dhs_data(countryIds = "SN", surveyYearStart = 2014, breakdown = "subnational", 
#>        returnGeometry = TRUE, f = "geojson") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_api_endpoints.R:60
#> 2: handle_api_request(endpoint, query, all_results, client, force) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/api_endpoints.R:135
#> 3: api_request(endpoint, query, all_results) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:48
#> 4: handle_pagination_geojson(endpoint, query, all_results) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/API.R:95
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | Authentication|  0     1 | Authentication|  0     2 | Authentication|  0     2 | Authentication
#> ─────────────────────────────────────────────────────────────────────────
#> test_authentication.R:5: skip: authenticate_dhs works
#> No authentication available
#> 
#> test_authentication.R:24: skip: available_surveys works
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | Client Setup|  0     1 | Client Setup|  0     2 | Client Setup|  0     2 | Client Setup
#> ─────────────────────────────────────────────────────────────────────────
#> test_client_set_up.R:5: skip: save credentials
#> No authentication available
#> 
#> test_client_set_up.R:30: skip: new updates are recognised
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | Downloads|  0     1 | Downloads|  0   1 1 | Downloads|  0 1 1 1 | Downloads|  0 1 2 1 | Downloads|  0 2 2 1 | Downloads|  0 2 3 1 | Downloads|  0 3 3 1 | Downloads|  0 3 4 1 | Downloads|  0 4 4 1 | Downloads|  0 4 4 2 | Downloads|  0 4 5 2 | Downloads|  0 5 5 2 | Downloads|  0 5 6 2 | Downloads|  0 6 6 2 | Downloads|  0 6 6 2 | Downloads
#> ─────────────────────────────────────────────────────────────────────────
#> test_downloads.R:4: skip: available surveys and download work
#> No authentication available
#> 
#> test_downloads.R:139: warning: ETAR71FL.ZIP test
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_downloads.R:139: error: ETAR71FL.ZIP test
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_downloads.R:139
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> 
#> test_downloads.R:148: warning: ugir41fl.zip test
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_downloads.R:148: error: ugir41fl.zip test
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_downloads.R:148
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> 
#> test_downloads.R:157: warning: zip file ending test
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_downloads.R:157: error: zip file ending test
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_downloads.R:157
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> 
#> test_downloads.R:168: warning: Hierarchal and sas7bdat dataset test
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_downloads.R:168: error: Hierarchal and sas7bdat dataset test
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_downloads.R:168
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> 
#> test_downloads.R:189: skip: Geo dataset test
#> No authentication available
#> 
#> test_downloads.R:214: warning: Large India Files
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_downloads.R:214: error: Large India Files
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_downloads.R:214
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> 
#> test_downloads.R:223: warning: Odd nesting India
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_downloads.R:223: error: Odd nesting India
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_downloads.R:223
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | Extraction|  0     1 | Extraction|  0   1 1 | Extraction|  0 1 1 1 | Extraction|  0 1 1 2 | Extraction|  0 1 1 2 | Extraction
#> ─────────────────────────────────────────────────────────────────────────
#> test_extraction.R:5: skip: query codes having downloaded surveys
#> No authentication available
#> 
#> test_extraction.R:109: warning: surveyId in extract
#> cannot open file 'rdhs.json': No such file or directory
#> 
#> test_extraction.R:109: error: surveyId in extract
#> cannot open the connection
#> 1: new_rand_client() at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_extraction.R:109
#> 2: rdhs::client_dhs(api_key = "ICLSPH-527168", config = read_rdhs_config_file("rdhs.json"), 
#>        root = td) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/helper_authentication.R:25
#> 3: read_rdhs_config_file("rdhs.json") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:18
#> 4: jsonlite::fromJSON(readChar(config_path, fsize), FALSE) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 5: readChar(config_path, fsize) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:283
#> 6: file(con, "rb")
#> 
#> test_extraction.R:125: skip: rbind_labelled
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | Miscellaneous utils|  1       | Miscellaneous utils|  2       | Miscellaneous utils|  3       | Miscellaneous utils|  4       | Miscellaneous utils|  5       | Miscellaneous utils|  6       | Miscellaneous utils|  7       | Miscellaneous utils|  8       | Miscellaneous utils|  8     1 | Miscellaneous utils|  9     1 | Miscellaneous utils|  9   1 1 | Miscellaneous utils| 10   1 1 | Miscellaneous utils| 10   1 1 | Miscellaneous utils [0.1 s]
#> ─────────────────────────────────────────────────────────────────────────
#> test_miscellaneous.R:38: skip: slow api response
#> No authentication available
#> 
#> test_miscellaneous.R:64: warning: different locales
#> OS reports request to set locale to "French_Belgium.1252" cannot be honored
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | Parse datasets|  1       | Parse datasets|  2       | Parse datasets|  3       | Parse datasets|  4       | Parse datasets|  5       | Parse datasets|  6       | Parse datasets|  7       | Parse datasets|  8       | Parse datasets|  9       | Parse datasets| 10       | Parse datasets| 11       | Parse datasets| 12       | Parse datasets| 13       | Parse datasets| 14       | Parse datasets| 15       | Parse datasets| 16       | Parse datasets| 17       | Parse datasets| 17     1 | Parse datasets| 17     1 | Parse datasets [6.9 s]
#> ─────────────────────────────────────────────────────────────────────────
#> test_read_dhs_flat.R:75: skip: lower case flat file check
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> |  0       | user interface|  0     1 | user interface|  0     2 | user interface|  0     3 | user interface|  0     4 | user interface|  0     5 | user interface|  0     6 | user interface|  0     7 | user interface|  0     8 | user interface|  0     9 | user interface|  0     9 | user interface
#> ─────────────────────────────────────────────────────────────────────────
#> test_ui.R:4: skip: check_for_client
#> No authentication available
#> 
#> test_ui.R:37: skip: check on the use of the default client for API caching in extract
#> No authentication available
#> 
#> test_ui.R:67: skip: get_datasets with no temp first
#> No authentication available
#> 
#> test_ui.R:97: skip: get_model_datasets
#> No authentication available
#> 
#> test_ui.R:107: skip: get_downloaded_datasets
#> No authentication available
#> 
#> test_ui.R:118: skip: get_available_datasets
#> No authentication available
#> 
#> test_ui.R:128: skip: search_and_extract_dhs
#> No authentication available
#> 
#> test_ui.R:146: skip: get_variable_labels
#> No authentication available
#> 
#> test_ui.R:168: skip: get_variable_labels direct via client
#> No authentication available
#> ─────────────────────────────────────────────────────────────────────────
#> 
#> ══ Results ══════════════════════════════════════════════════════════════
#> Duration: 310.6 s
#> 
#> OK:       56
#> Failed:   8
#> Warnings: 8
#> Skipped:  21

Created on 2018-07-26 by the reprex package (v0.2.0).

questions

  • There still seems to be tests that are failing. Are these primarily still because of authentication issues? My understanding was that you are skipping any tests that require authentication? Should these not be skipping?
  • Re: getting me authentication, it is never recommended to share your own private key with anyone. What would be better is that I get an authorisation token for myself. I imagine any user of the package would need this too? In which case, the README should flag this and also include instructions on how to get an authorisation token and use it to access the API. You could also include a whole separate vignette for this but it should still be flagged and linked to prominently in the README

@OJWatson
Copy link
Author

Thank you for the reply @annakrystalli and sorry about these failing tests.

  • 7 out of 8 fails/warnings were for where I had failed to place skip_if_no_auth calls at the beginning of tests within test_downloads and test_extraction so these were failing due to having no authentication.

  • The other test in test_api_endpoints was failing as returning geojson objects from their API is currently really slow due to the size of the objects returned, and frequently doesn't return correctly and subsequently freezes their API up for a bit. This is why that's the only test that has a skip_on_travis as well. They are working on improving the speed/reliability on their API but for the time being i've just skipped this test.

  • Re: auth, apologies again as that last message wasn't super clear. No one needs a key for the API as the DHS Programme have said it is okay for the API-key they have given us to be used by all users of the package. So all API tests (except for that geojson one above) should work fine without any authorisation by the user.

  • Users will, however, need to create an account with the DHS website though if they want to download the survey datasets from the website. This is the info that is stored in the encrypted rdhs.json.tar.enc file. The key I mentioned earlier is to decrypt the rdhs.json.tar.enc file in the github repo, rather than my key for downloading data sets from their website. So if someone wants to run all the tests locally, they will need to have created an account with the DHS website, and then set their rdhs_config using set_rdhs_config(global=FALSE) to create the "rdhs.json" file. I'll make a small vignette about this, as I haven't described how to run all the tests locally anywhere in the pkgdown site and while writing this out i've realised this is not very clear. Will post here when i've done it.

Thank you again for this and sorry about the tests,

OJ

@annakrystalli
Copy link
Contributor

Hi OJ,

OK thanks for the clarification, that all makes more sense now but yeah I think a short vignette would be really useful. Do you think you get something sorted soonish for the reviewers to have a look at?

Re: the tests, no worries, that's what the review process is all about, ironing out the kinks! :)

Regarding:

their API is currently really slow due to the size of the objects returned, and frequently doesn't return correctly and subsequently freezes their API up for a bit.

I was wondering if maybe this sort of failure could be handled more explicitly, ie by throwing an informative error regarding why it failed?

Otherwise, I'm happy to start seeking reviewers if you are?

@OJWatson
Copy link
Author

Hi,

  • I have just added another vignette on this here, which is linked to in the readme.

  • I've also added some bubble-wrapping around the API tests, so that if they haven't responded after 30 seconds (default timeout) then these tests are skipped. And then not in tests, these will display more helpful error messages about why.

And yep happy for reviewers.

Thank you again for all the help,

OJ

@annakrystalli
Copy link
Contributor

annakrystalli commented Aug 7, 2018

Good news @OJWatson, we have reviewers! 🎉
Many thanks @dosgillespie & @czeildi for agreeing to review.


Reviewers: @dosgillespie @czeildi
Due date: 2018-08-28

@czeildi
Copy link

czeildi commented Aug 23, 2018

Hi,

@OJWatson I have one small remark: the NEWS could be more concise, more shorter bullet points grouped according to new function / bug fix etc would make it easier to skim through it. You may consider getting ideas from http://style.tidyverse.org/news.html.

FYI I won't be reviewing the package as discussed with Anna.

@OJWatson
Copy link
Author

Hi,

Thanks for this @czeildi - have never written a NEWS section before, and reading it back I have definitely waffled. I'll trim and tidy it up in line with the style guide you've linked.

Many thanks, OJ

@annakrystalli
Copy link
Contributor

Hi @OJWatson, just to keep you up to date, I have been trying to find a new reviewer, hoping it wouldn't take too long but alas, it has. I'm still on the case but as I'm still on the case I will return the status of the review to seeking reviewers.

I'll also reset the deadline for review when a new reviewer has been found. Sorry about this! If you have any suggestions for a reviewer let me know!

@annakrystalli
Copy link
Contributor

annakrystalli commented Sep 3, 2018

Great news! We finally have two reviewers again! 🎉
Thanks again to both for agreeing to review 🙏


Reviewers: @dosgillespie @LucyMcGowan
Due date: 2018-09-24 2018-09-30 (extended)

@OJWatson
Copy link
Author

OJWatson commented Sep 4, 2018

Yay! Thanks very much for finding someone so quickly @annakrystalli, and thank you to @LucyMcGowan and @dosgillespie for reviewing.

I've also updated/tidied the NEWS in keeping more with the tidyverse style guide.

@LucyMcGowan
Copy link

LucyMcGowan commented Sep 18, 2018

Package Review

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

##### For packages co-submitting to JOSS

- [ ] The package has an obvious research application according to JOSS's definition

The package contains a paper.md matching JOSS's requirements with:

- [ ] A short summary describing the high-level functionality of the software
- [ ] Authors: A list of authors with their affiliations
- [ ] A statement of need clearly stating problems the software is designed to solve and its target audience.
- [ ] References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2.5


Review Comments

Summary

The rdhs package facilitates querying the Demographic and Health Survey API and makes downloading and interacting with the raw datasets a smoother, more reproducible process. This looks great!

A few things:

  1. I didn't run the full test suite, because I didn't request access in time (it looks like it takes a few days to approve, I am having a 👶 sometimes this week (🤞) and I wanted to be sure to get this review in). It looks like others have been able to run successfully, though.

  2. Of the tests that ran, I had two failures -- I believe because I opted out of having the results cached.

 1. Error: format catches and all_results tests (@test_api_endpoints.R#11) 
 2. Error: password obscure (@test_miscellaneous.R#77) 

Would you consider having an additional skip condition for this circumstance?

  1. Many of the functions don't have examples in the documentation -- it would be great to have a few more that include examples. The ones that do have examples, it is not always clear to me what the example is doing. For example, here is the examples section from dhs_data():
## not run only because they take a very long time and interact with an API
## Not run: 
dat <- dhs_data(countryIds="EG",all_results=FALSE)
dat <- dhs_data(indicatorIds="FE_FRTR_W_TFR",all_results=FALSE)
dat <- dhs_data(surveyIds="SN2010DHS",all_results=FALSE)
dat <- dhs_data(selectSurveys="latest",all_results=FALSE)
dat <- dhs_data(selectSurveys="byIndicator", indicatorIds="FE_CEBA_W_CH0",
all_results=FALSE)
dat <- dhs_data(surveyYear="2010",all_results=FALSE)
dat <- dhs_data(surveyYearStart="2006",all_results=FALSE)
dat <- dhs_data(surveyYearStart="1991", surveyYearEnd="2006",
all_results=FALSE)
dat <- dhs_data(surveyType="DHS",all_results=FALSE)
dat <- dhs_data(surveyCharacteristicIds="32",all_results=FALSE)
dat <- dhs_data(characteristicCategory="wealth quintile",all_results=FALSE)
dat <- dhs_data(breakdown="all", countryIds="AZ", characteristicLabel="6+",
all_results=FALSE)
dat <- dhs_data(tagIds="1",all_results=FALSE)
dat <- dhs_data(breakdown="subnational",all_results=FALSE)
dat <- dhs_data(breakdown="background",all_results=FALSE)
dat <- dhs_data(breakdown="all",all_results=FALSE)
dat <- dhs_data(f="html",all_results=FALSE)
dat <- dhs_data(f="geojson", returnGeometry="true",all_results=FALSE)

## End(Not run)

This seems like you're showing lots of different ways to run this function, but what each one does is not explained. I would maybe include far fewer lines (maybe just the ones you think would be the most common) along with comments to explain what output is expected (this applies to a lot of the functions, I'm just showing the one here).

  1. You have a message that states:
Writing your configuration to:
   -> /var/folders/9x/fpr7_fbn4ln194by50dnpgyw0000gn/T//RtmpQO2pa4/rdhs/rdhs.json

If you are using git, be sure to add this to your .gitignore

You may consider adding this to the .gitignore for the user? For example httr does this with an add_line() function: https://github.com/r-lib/httr/blob/976289a3596dc01dc994f8fd743770a172bbecdb/R/oauth-cache.R#L61-L75
and then adding the config file like this: https://github.com/r-lib/httr/blob/976289a3596dc01dc994f8fd743770a172bbecdb/R/oauth-cache.R#L61-L75

  1. In the Introduction, section 3 outlines how to download survey datasets. This involves "providing as arguments your email and project for which you want to download datasets from. You will then be prompted for your password." This is the first time a password is mentioned in this document -- as someone not familiar with the interface, I was confused about where I needed to obtain this password -- maybe a link to the website would be helpful here?

  2. I caught a few typos, I submitted this PR: A few small typos #48

@OJWatson
Copy link
Author

Hi there @LucyMcGowan,

Thank you so much for the review, especially with your 👶 due so soon. Hope everything goes well.

I will respond fully by the end of today, with my changes but I agree with everything that you have suggested, in particular making sure everything has an example and then shortening the examples that the API functions run to just one function call.

Thank you again,

OJ

@annakrystalli
Copy link
Contributor


Due date: 2018-09-24 2018-09-30 (extended)

@OJWatson
Copy link
Author

Nearly finished - some of the examples have proven more tricky to write so that they run due to auth issues etc, but am nearly there.

@OJWatson
Copy link
Author

Hi @LucyMcGowan,

Finally got there with the examples. So working through the comments.

  1. Hope everything is going okay with 👶 and thank you again for finding the time for the review.

  2. The test suite used to fail if you had not created a valid "rdhs.json" config file. I've touched the API tests won't fail by first creating a config if not found, as these tests can run without a DHS website account. This is included in commit d66c8e

The other tests that require a login, however, will still require people to follow the testing vignette here

  1. There are now examples for all the exported functions, where they are needed. Where possible, these examples are run using the model datasets that the DHS website publishes, which anyone can download. This way you can use the main function in ui.R, e.g. get_downloaded_datasets
#' # get the model datasets included with the package
#' model_datasets <- model_datasets
#'
#' # download one of them
#' g <- get_datasets(dataset_filenames = model_datasets$FileName[1])
#'
#' # these will then be stored so that we know what datasets we have downloaded
#' d <- get_downloaded_datasets()

With regards to the examples for the API functions, the long list of examples that you included for dhs_data was in there as it is a copy of all the examples that their API documentation provide, so I thought it would be nice to show how each one would be carried out using rdhs. I agree though this is not helpful, so I have added an initial couple of examples of common/useful use cases for each function with more descriptions as they go along. Then I explain what the long list of examples is so that people can link back to the DHS API website. For example for dhs_ui_updates

#' # The main use for the ui updates API will be to search for the last time
#' # there was a change to the UI. For example to return all the
#' # changes since 2018:
#'
#' dat <- dhs_ui_updates(lastUpdate="20180101")
#'
#' # A complete list of examples for how each argument to the ui updates API
#' # endpoint can be provided is given below, which is a copy of each of
#' # the examples listed in the API at:
#'
#' # https://api.dhsprogram.com/#/api-uiupdates.cfm
#'
#'
#' dat <- dhs_ui_updates(lastUpdate="20150901",all_results=FALSE)
#' dat <- dhs_ui_updates(f="html",all_results=FALSE)
#' }

I have, however, put these examples within \dontrun{} as their API can be temperamental.

  1. Thanks for linking httr's way of doing this as this is much neater. Have implemented as well as adding it to the .Rbuildignore. This is included in commit 1ba973.

  2. Completely agree. I have added an initial paragraph in both the README and the introduction vignette for this, which was included in commit 6c5554

  3. Thank you picking these out and double thank you for making a PR with them.

I think that's everything. I've also made a few changes outside these comments, which clear up some of the issues that were outstanding and have added another example of how to use the "geojson" return type from API requests to build maps, shown here


Thank you again for the kind words about the package and for the comments and the suggestions. Definitely helped increase the usability of the package and thanks for linking the add_line function from httr, as i end up wanting that functionality a lot.

All the best,

OJ

@annakrystalli
Copy link
Contributor

annakrystalli commented Sep 30, 2018

Hi @OJWatson, thanks for your swift response to @LucyMcGowan 's comments (and thanks @LucyMcGowan for them and your contributions, hope all is going well!). The package and documentation are really shaping up really nicely.

Re (5)

  • Because downloading data is such an important functionality, I feel introducing the user to the fact that they will have to register an rdhs account would be appropriate very early on, in the README installation instructions, by including:

    To be able to download survey datasets from the DHS website, you will need to set up an account with them to enable you to request access to the datasets. Instructions on how to do this can be found here

  • Then, for more appropriate signposting to the testing vignette, I think a summary of testing environment setup, with a link to the full testing vignette should be included in a brief testing section of a CONTRIBUTING.md. Here's a template you can adapt. Sorry, I should have picked this up a bit earlier!. Depending on how likely you think users will need to know about the extra setup for testing, you could remove mention to it in the README, perhaps adding instead a mention and link at the bottom to the CONTRIBUTING.md guidelines. Or both.

Thoughts?

@OJWatson
Copy link
Author

Hey,

Thanks for taking the time to go through this issue so thoroughly. I have been able to replicate the issue using the oldrel in travis (https://travis-ci.org/OJWatson/rdhs/jobs/459793328#L4126). 🙌 (P.S. Thanks for pointing out oldrel as i had not seen this before).

We will be moving towards using haven more than foreign, so I have changed the default function used to read in .dta files to be haven::read_dta rather than the equivalent in foreign. This addresses that test error. Also I think since we use the haven::labelled format for reading in files as well, it makes sense to use haven for default options of reading data sets, while also providing people who have downstream pipelines using foreign. This is because I had to make a number of patches to address the changes in haven v 2.0.0 which dropped on CRAN 4 days ago. So we're going full haven 🏠

Re: tempdir_check - I think the user simply deleted the temp directory using a file browser, so definitely falls under the "better practice from the user" heading. The result was then that if rdhs tried to set up the config in the temporary directory (the default if they have not given permission to R to write outside of their temporary directory), it was trying to save files etc to a missing directory. Since this is not an rdhs issue but more a user issue, I've reverted this back to just using tempdir as normal.

Thanks again for all the help, OJ

@annakrystalli
Copy link
Contributor

OK, back with you.

So I pulled your changes and ran the tests. Getting a new failure and a couple of warnings (edited down):

devtools::test("/Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs")
#> Loading rdhs
#> Loading required package: testthat
#> Testing rdhs
───────────────────────────────────────────────────────────────────────────────────
⠏ |  0       | Miscellaneous utils|  1       | Miscellaneous utils|  2       | Miscellaneous utils|  3       | Miscellaneous utils|  4       | Miscellaneous utils|  5       | Miscellaneous utils|  6       | Miscellaneous utils|  7       | Miscellaneous utils|  8       | Miscellaneous utils|  8     1 | Miscellaneous utils|  9     1 | Miscellaneous utils|  9   1 1 | Miscellaneous utils| 10   1 1 | Miscellaneous utils| 10   1 2 | Miscellaneous utils| 10   1 3 | Miscellaneous utils| 10   2 3 | Miscellaneous utils| 10 1 2 3 | Miscellaneous utils| 10 1 2 3 | Miscellaneous utils

1. The warning re: locale again:

#> test_miscellaneous.R:64: warning: different locales
#> OS reports request to set locale to "French_Belgium.1252" cannot be honored

I had a little play around with this and Im not sure whether it's testing what you want it to? ie on my system it fails to set the locale during testing (indeed it doesn't change the locale even if I run Sys.setlocale("LC_TIME","French_Belgium.1252") in the console. The test doesn't fail though because the date remains parsable by mdy_hms(date). Could you elaborate a little more on what it is you are trying to test here?

2. Warning re: object 'model_datasets'

#> test_miscellaneous.R:114: warning: model datasets onAttach
#> object 'model_datasets' not found

I should note however that I don't get these warnings when running devtools::check(). Will seek some advice on this.

3.Error : looks like another one requiring credentials that needs to be skipped

#> test_miscellaneous.R:115: error: model datasets onAttach
#> Login credentials for the DHS website have not been found.Please uset set_dhs_config() and provide your DHS email and project.
#> 1: rdhs::get_datasets("MWAR7ASV.ZIP") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/tests/testthat/test_miscellaneous.R:115
#> 2: client$get_datasets(dataset_filenames, download_option = download_option, reformat = reformat, 
#>        all_lower = all_lower, output_dir_root = output_dir_root, clear_cache = clear_cache, 
#>        ...) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/ui.R:128
#> 3: handle_config(private$config$config_path) at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/client.R:345
#> 4: stop("Login credentials for the DHS website have not been found.", "Please uset set_dhs_config() and provide your DHS email and project.") at /Users/Anna/Documents/workflows/rOpenSci/editorials/rdhs/R/config.R:35

#> ══ Results ════════════════════════════════════════════════════════════════════════
#> Duration: 46.8 s
#> 
#> OK:       59
#> Failed:   1
#> Warnings: 2
#> Skipped:  29

Created on 2018-11-29 by the reprex package (v0.2.0).

@OJWatson
Copy link
Author

Hi again, and sorry for the new errors :(

1. re: locale

Some users' OS may have different locale environment variable LC_TIME, which for one user in this issue lead to them being unable to parse date strings that were returned from the DHS API. So we have a function mdy_hms, which will change their LC_TIME env. variable to be equal to C for the purposes of parsing dates from the API. The following reprex (run on a windows)(should kind of show the problem a bit clearer a bit more information about what the locale change is for:

library(rdhs)

# the API returns dates in the following string
date <- "July, 15 2016 19:17:14"

# these will be correctly parsed if LC_TIME recognises it
strptime(date, format = "%B, %d %Y %H:%M:%S")
#> [1] "2016-07-15 19:17:14 UTC"

# which in this session is
locale_lc_time <- Sys.getlocale("LC_TIME")
locale_lc_time
#> [1] "English_United Kingdom.1252"

# if however it does not, e.g French_Belgium.1252
Sys.setlocale("LC_TIME","French_Belgium.1252")
#> [1] "French_Belgium.1252"
Sys.getlocale("LC_TIME")
#> [1] "French_Belgium.1252"
strptime(date, format = "%B, %d %Y %H:%M:%S")
#> [1] NA

# our function mdy_hms will change your LC_TIME to C before using strptime
mdy_hms(date)
#> [1] "2016-07-15 19:17:14 UTC"

# but will not change LC_TIME
Sys.getlocale("LC_TIME")
#> [1] "French_Belgium.1252"

The above, however, I was only able to do on my windows machine, and on my linux machine I get the same cannot be honoured message as you do. As a result, I've wrapped the test in a tryCatch to catch for a warning related to this, and to skip that test if it does throw the warning, i.e. the OS can't handle it.

2 & 3:

These both refer to a test I added recently, which was to address an issue where I had a forgotten to make model_datasets, which is a core data set the package uses when downloading data sets, an internal data set as well as one that should be exposed to the end user for demo purposes etc. This meant that a lot of functions would not work without rdhs being attached. I've slightly tweaked the test to better test this, and put an authentication skip on it as it does require auth.


I've just pushed these changes now into the master branch as well, so hopefully 🤞 these tests should then be handled okay now (famous last words)

Thanks again for everything, OJ

@annakrystalli
Copy link
Contributor

famous last words

...indeed! 😜

So, I asked for some advice from the rest of the editors and they hit up against another error when running the tests for the first time:

── 1. Error: post api tidy (@test_api_endpoints.R#354)  ────────────────────────
object 'conf' not found
1: .handleSimpleError(function (e)
   {
       handled <<- TRUE
       test_error <<- e
       options(expressions = expressions_opt_new)
       on.exit(options(expressions = expressions_opt), add = TRUE)
       e$expectation_calls <- frame_calls(11, 2)
       test_error <<- e
       register_expectation(e)
       e$handled <- TRUE
       test_error <<- e
   }, "object 'conf' not found", quote(eval(code, test_env))) at testthat/test_api_endpoints.R:354

Somehow I don't get this error, not sure why because I haven't set up credentials and get_rdhs_config() returns NULL for me. In any case, looking at the test itself I think there might be an error:

test_that("post api tidy", {

  if (file.exists("rdhs.json")) {
  conf <- read_rdhs_config_file("rdhs.json")
  } else {
    expect_true(TRUE)
  }

  if (is.null(conf$email)){
    expect_true(file.remove("rdhs.json"))
  }

})

Feels like

 if (is.null(conf$email)){
    expect_true(file.remove("rdhs.json"))
  }

shouldn't run if file.exists("rdhs.json") == FALSE right? At the minute, even if the file doesn't exist in the first place and hence there is no conf object to interogate for conf$email, it still tries to, hence the error object 'conf' not found. So I think this needs correcting.

Interestingly, the secomd time they ran the tests, this error didn't turn up! Does the initial run create and cache a rdhs.json file somewhere? Perhaps after this particular test is run?

@annakrystalli
Copy link
Contributor

BTW... just pulled and ran check() and test() and all clear!! 🙌

@annakrystalli
Copy link
Contributor

I'll be back tomorrow morning to do another quick pass and hopefully get everything approved! 😃

@OJWatson
Copy link
Author

Wooop! Okay and yes the above makes sense.

The API tests in test_api_endpoints will create an rdhs.json file in the current directory if there isn't one. Though if there wasn't one it won't have anything stored for email project password etc. So this tidy up was to make sure that any rdhs.json file created by the API tests is deleted if it will not be useful for the following tests that require auth. So i do a check on conf$email.

However, the following should have been inside the first if statement like so:

  if (file.exists("rdhs.json")) {
    conf <- read_rdhs_config_file("rdhs.json")
    if (is.null(conf$email)){
      expect_true(file.remove("rdhs.json"))
    }
    
  } else {
    expect_true(TRUE)
  }

However, I am a little confused though how they would have gotten to that test without creating an rdhs.json. The only thing I can guess is that the API was currently down when they ran the tests, so all the API tests above that test would have been skipped and thus they wouldn't have made the "rdhs.json" file. Either way the changes above should then catch for that now, and these have been pushed.

@annakrystalli
Copy link
Contributor

Hey OJ,

Just been having a final review and look around the package, I made a small PR with some minor edits to the README, have a look and let me know what you think.

There's only one last point I want to raise. In client_dhs(), the API key is baked into the function as a default to argument api_key. While I appreciate it's a public key and see no reason for it not to be included in the source code, I feel a bit uncomfortable about it being so prominently user-facing. I think at some point I had even seen it turn up in errors when the function failed. I feel a better approach would be to set the default to NULL and then internally set it to the default one if no other API key is supplied to that argument.

Otherwise, I think we are good to 🚀 😃

@OJWatson
Copy link
Author

OJWatson commented Dec 3, 2018

Hi,

Completely agree - I have moved the actual string to be an internal package data object, and have removed all occurrences of it being written down explicitly in the code (although it is still in old commits).
As an aside the DHS programme said they wanted us to ship the package with that API key so that they could see how many people were using rdhs etc, but making it harder for people to get it makes sense.

Thanks again for everything,

OJ

@annakrystalli
Copy link
Contributor

Approved! 🙌 You're a 🌟. Excellent work on this @OJWatson! We're finally there 😃

Thanks for submitting and thanks @LucyMcGowan & @dosgillespie for your reviews!

To-dos:

  • Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Add the rOpenSci footer to the bottom of your README
    " [![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)"
  • Fix any links in badges for CI and coverage to point to the ropensci URL. We no longer transfer Appveyor projects to ropensci Appveyor account so after transfer of your repo to rOpenSci's "ropensci" GitHub organization the badge should be [![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/github/ropensci/rdhs?branch=master&svg=true)](https://ci.appveyor.com/project/OJWatson/rdhs).
  • We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.

For JOSS submission

Once admin rights have been returned:

  • Activate Zenodo watching for the repo
  • Tag and create a release so as to create a Zenodo version and DOI
  • Submit to JOSS using the Zenodo DOI. We will tag it for expedited review.

Should you want to awknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent). More info on this here. If you choose to, I suggest you do this before creating your codemeta file.

Welcome aboard! We'd also love a blog post about your package, either a short-form intro to it (https://ropensci.org/tech-notes/) or long-form post with more narrative about its development. (https://ropensci.org/blog/). If you are interested, @stefaniebutland will be in touch about content and timing.

We've started putting together a gitbook with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here.

Finally, if you're planning to submit to CRAN, have a look at the section about CRAN gotchas in the onboarding guide. And feel free to reach out if you need help through the process.

Well done again @OJWatson 👍

@OJWatson
Copy link
Author

OJWatson commented Dec 4, 2018

Yay! 🎉 - Thank you so much @annakrystalli for the help in these last stages. I've just transferred the repo over and will start addressing the links etc.

Thank you again to @LucyMcGowan and @dosgillespie for taking the time to review it. I would love to acknowledge both of you as reviewers in the DESCRIPTION if that is okay with both of you? Please let me know if that's okay, and also if you would like me to link your ORCID profile if you have one/would like it linked.

Also just wanted to say how helpful and nice the whole process has been. I've learnt loads and have picked up a number of really useful things during the review cycle. It's a really great initiative so thanks again to everyone.

@OJWatson
Copy link
Author

OJWatson commented Dec 4, 2018

Also @stefaniebutland - I'm definitely keen to write a blog post. Haven't quite decided what form but should be able to mock something up in the next week or so if that's okay?

@annakrystalli
Copy link
Contributor

😱Somehow I managed to miss asking you to add the rOpenSci review status badge to the README! 🤦‍♀️. Could I get you to add it now please? 🙏

[![](https://badges.ropensci.org/238_status.svg)](https://github.com/ropensci/software-review/issues/238)

In other news, admin rights have now been returned! Just ping me when you've submitted to JOSS so I can tag it as reviewed here.

@OJWatson
Copy link
Author

OJWatson commented Dec 4, 2018

Hi again,

Have added the banner. Sorry for delay - had it sitting in the patching branch.

Also do you know how to get github to stop declaring the package's main code language as html? It keeps picking up the files in the docs folder, even though .gitattributes has specified docs/* linguist-vendored?

Many thanks again,

OJ

@annakrystalli
Copy link
Contributor

Hi OJ, my guess (without digging into the history) is that the addition of the .gitattributes line came after the first commit of the docs/. Have a look at this comment and associated example PR which describes the situation and how to correct it (I believe you just need to delete all docs, rebuild and recommit them for the linguist detector to ignore them). Let me know if that works!

@OJWatson
Copy link
Author

OJWatson commented Dec 5, 2018

👍 - thank you. think it was both the deleting and the paper.html file that were causing this. Thanks again.

@stefaniebutland
Copy link
Member

I'm definitely keen to write a blog post.

That's great to hear @OJWatson.
We could publish 2019-01-03 (Thursday 3rd) - the first post of the new year 🎉. That would mean submitting a draft for review by 2018-12-17 since I'm trying not to schedule any review work between Christmas and New Year.

This link will give you many examples of blog posts by authors of peer-reviewed packages so you can get an idea of the style and length you prefer: https://ropensci.org/tags/software-peer-review/.

Here are some technical and editorial guidelines for contributing a blog post: https://github.com/ropensci/roweb2#contributing-a-blog-post.

Happy to answer any questions.

@annakrystalli
Copy link
Contributor

Hey @OJWatson! 👋

Any chance you could update the post-review checklist above so I can see where we are at? 🙏

@OJWatson
Copy link
Author

Hi,

Apologies for missing that!:

To-dos:

  • Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Add the rOpenSci footer to the bottom of your README
  • Fix any links in badges for CI and coverage to point to the ropensci URL. We no longer transfer Appveyor projects to ropensci Appveyor account so after transfer of your repo to rOpenSci's "ropensci" GitHub organization the badge should be
  • We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.

For JOSS submission

Once admin rights have been returned:

  • Activate Zenodo watching for the repo
  • Tag and create a release so as to create a Zenodo version and DOI
  • Submit to JOSS using the Zenodo DOI. We will tag it for expedited review.

^ I'm not planning on submitting to JOSS.


Sorry about missing that again - let me know if there is anything else,

OJ

@annakrystalli
Copy link
Contributor

annakrystalli commented Dec 10, 2018

Great thanks for that @OJWatson ! OK so last thing is to just add the rOpenSci badge to the README. (Sorry, this should have been done ages ago but I missed it)

[![](https://badges.ropensci.org/238_status.svg)](https://github.com/ropensci/software-review/issues/238)

Just ping when it's done and I'll close the issue 🙌😃

@OJWatson
Copy link
Author

Sorry - that's my bad. I got confused between the footer and the badge. I've now added the badge to the readme.

@annakrystalli
Copy link
Contributor

All good! I thought as much 😉

Well done again 💯

@stefaniebutland
Copy link
Member

Hi @OJWatson. I'm following up to see if you'd still like to contribute a blog post and to agree on a submission/publication date.

I'll be on vacation Dec 24 - Jan 1, so I could review a draft this week if submitted by Fri Dec 21, 9am Pacific, or review in the new year. If you are still interested, please date your post 2019-01-03 and we can modify that once it's ready.

This link will give you many examples of blog posts by authors of peer-reviewed packages so you can get an idea of the style and length you prefer: https://ropensci.org/tags/software-peer-review/.

Here are some technical and editorial guidelines for contributing a blog post: https://github.com/ropensci/roweb2#contributing-a-blog-post.

Happy to answer any questions.

@stefaniebutland
Copy link
Member

Happy New Year @OJWatson. If you are still interested in writing a post about rdhs, I'm still interested in helping you get more eyes on your work 🙂. Please let me know at any time and we can establish a publication date that works for you.

@OJWatson
Copy link
Author

OJWatson commented Jan 4, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants