search

ropensci / software-review

rOpenSci Software Peer Review.
286 stars 104 forks source link

eph: Caja de Herramientas para el procesamiento de la Encuesta Permanente de Hogares #593

Closed caropradier closed 9 months ago

caropradier commented 1 year ago

Date accepted: 2023-09-14

Nombre de la Persona Encargada: Carolina Pradier Usuario GitHub de la Persona Encargada: !--author1-->@caropradier<!--end-author1-- Usuario GitHub de las Otras Autoras del Paquete: (borra si solo hay una autora) @DiegoKoz, @pablotis,@Guidowe,@natsushok,@gefero,@jgjuara Repositorio: https://github.com/holatam/eph Versión Enviada: Tipo de Envio: Estándar Editora: !--editor-->@maurolepore<!--end-editor-- Revisores: @guadag12, @lidefi87

Archivo: TBD Versión Aceptada: TBD Idioma: es

Package: eph
Title: Argentina's Permanent Household Survey Data and Manipulation Utilities
Version: 0.6.0
Authors@R: c(
    person("Diego", "Kozlowski", email = "diegokoz92@gmail.com", role = c("aut"), comment = c(ORCID = "0000-0002-5396-3471")),
    person("Pablo", "Tiscornia", email = "pablotisco@gmail.com", role = "aut"),
    person("Guido", "Weksler", email = "guidowe45@gmail.com", role = c("aut")),
    person("Natsumi", "Shokida", email = "natsumi.shokida@gmail.com", role = "aut"),
    person("German", "Rosati", email = "german.rosati@gmail.com", role = "aut", comment = c(ORCID = "0000-0002-9775-0435")),
    person("Juan Gabriel", "Juara", email = "jg.juara@gmail.com", role = "ctb"),
    person("Carolina", "Pradier", email = "carolinapradier@gmail.com", role = c("aut","cre"), comment = c(ORCID = "0009-0007-5058-6352")))
Description: Tools to download and manipulate the Permanent Household Survey from Argentina
    (EPH is the Spanish acronym for Permanent Household Survey).
    e.g: get_microdata() for downloading the datasets, get_poverty_lines() for downloading the official poverty baskets,
    calculate_poverty() for the calculation of stating if a household is in poverty or not, following the official methodology.
    organize_panels() is used to concatenate observations from different periods, and organize_labels()
    adds the official labels to the data. The implemented methods are based on INDEC (2016) <http://www.estadistica.ec.gba.gov.ar/dpe/images/SOCIEDAD/EPH_metodologia_22_pobreza.pdf>.
    As this package works with the argentinian Permanent Household Survey and its main audience is from this country,
    the documentation was written in Spanish.
BugReports: https://github.com/holatam/eph/issues
Imports:
    dplyr,
    expss,
    purrr,
    tibble,
    stringr,
    readxl,
    tidyr,
    utils,
    attempt,
    zoo,
    leaflet,
    htmltools,
    rlang,
    rvest,
    xml2,
    stats,
    cli
Depends: R (>= 2.10)
License: MIT + file LICENSE
Encoding: UTF-8
Language: es
URL: https://github.com/holatam/eph
LazyData: true
RoxygenNote: 7.2.3
Suggests: 
    testthat,
    lubridate,
    knitr,
    rmarkdown,
    ggplot2,
    readr,
    forcats
VignetteBuilder: knitr

Alcance

La librería eph tiene como objetivo facilitar el trabajo con las bases de datos vinculadas a la Encuesta Permanente de Hogares - INDEC. Incluye herramientas para descargar las bases de datos, etiquetar y recodificar sus variables y acceder a datasets complementarios.

Científicos sociales y otros usuarios de la EPH que utilizan sus datos para elaborar investigaciones científicas, informes de coyuntura, recomendaciones de política pública, etc.

Hay otros paquetes de R que facilitan la descarga y procesamiento de otras bases de datos (e.j. ech, PNADclBGE, readabs), pero ninguno de ellos es aplicable a la EPH.

Si

Nota del editor (@maurolepore): Este paquete es parte del "Champions Program". @yabellini esta al tanto de su envio. Yo tambien soy parte del programa y el potencial conflicto de interes fue discutido y aprobado por @noamross.

Editor's note (@maurolepore). This package is part of the "Champions Program". @yabellini is aware of this submission. I am also part of the program and the potential conflict of interest was discusses and approved by @noamross.

Revisiones Técnicas

Tilda los siguientes items para confirmar que los has completado:

Este paquete:

Opciones de Publicación

Opciones para MEE - [ ] Este paquete es novedoso y será de interés para la mayoría de lectores de la revista. - [ ] El manuscrito que describe el paquete no tiene más de 3000 palabras y está escrito en Inglés. - [ ] Tienes intenciones de archivar el código del paquete en un repositorio a largo plazo, que cumple los requerimientos de la revista (mira las [Políticas de Publicación de MEE (documento en Inglés)](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/journal-resources/policy-on-publishing-code.html)) - (*Alcance: Considera los [Objetivos y Alcance de MEE (documento en Inglés)](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/aims-and-scope/read-full-aims-and-scope.html) para tu manuscrito. No otorgamos garatías de que tu manuscrito esté en el ámbito de MEE.*) - (*Aunque no es requerido, recomendamos tener un manuscrito completamente preparado y en Inglés, al momento de enviar.*) - (*Por favor, no envíes tu paquete de forma separada a Methods in Ecology and Evolution*)

Código de Conducta

ropensci-review-bot commented 1 year ago

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

ropensci-review-bot commented 1 year ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 1 year ago

Checks for eph (v0.6.0)

git hash: 707cd24c

(Checks marked with :eyes: may be optionally addressed.)

Package License: MIT + file LICENSE

1. Package Dependencies

Details of Package Dependency Usage (click to open)

The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate. |type |package | ncalls| |:----------|:---------|------:| |internal |base | 334| |internal |eph | 20| |internal |methods | 4| |internal |graphics | 1| |imports |utils | 83| |imports |dplyr | 50| |imports |purrr | 29| |imports |stats | 17| |imports |tidyr | 16| |imports |cli | 12| |imports |attempt | 4| |imports |tibble | 3| |imports |stringr | 3| |imports |readxl | 3| |imports |leaflet | 3| |imports |rvest | 2| |imports |zoo | 1| |imports |htmltools | 1| |imports |xml2 | 1| |imports |expss | NA| |imports |rlang | NA| |suggests |testthat | NA| |suggests |lubridate | NA| |suggests |knitr | NA| |suggests |rmarkdown | NA| |suggests |ggplot2 | NA| |suggests |readr | NA| |suggests |forcats | NA| |linking_to |NA | NA| Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats()', and examining the 'external_calls' table.

base

c (181), list (14), unz (14), ncol (11), by (10), try (10), names (9), sprintf (8), data.frame (7), round (7), readRDS (6), sum (5), grep (4), ifelse (4), substr (4), colnames (3), colSums (3), is.null (3), mode (3), suppressWarnings (3), t (3), tempfile (3), emptyenv (2), gzcon (2), local (2), paste0 (2), url (2), as.character (1), I (1), invokeRestart (1), lapply (1), paste (1), rowSums (1), sin (1), toupper (1), warning (1)

utils

de (58), read.table (14), download.file (6), data (3), unzip (2)

dplyr

mutate (24), select (6), case_when (4), left_join (4), add_row (2), summarise (2), across (1), as_label (1), bind_cols (1), bind_rows (1), inner_join (1), mutate_at (1), n (1), transmute (1)

purrr

pmap (12), map (6), map_lgl (3), pluck (3), safely (3), as_vector (2)

eph

is_online (4), epyh (2), get_eahu_internal (2), get_microdata_internal (2), calculate_errors (1), calculate_poverty (1), calculate_tabulates (1), errores_muestrales (1), find_closest (1), get_eahu (1), get_microdata (1), get_poverty_lines (1), get_total_urbano_internal (1), Teuno (1)

stats

df (7), D (3), C (2), xtabs (2), dt (1), weights (1), window (1)

tidyr

pivot_wider (10), pivot_longer (3), unnest (3)

cli

cli_abort (12)

attempt

stop_if_not (4)

methods

el (3), as (1)

leaflet

addCircleMarkers (1), addLegend (1), colorNumeric (1)

readxl

read_excel (3)

stringr

str_pad (3)

tibble

tibble (3)

rvest

html_attr (1), html_nodes (1)

graphics

title (1)

htmltools

HTML (1)

xml2

read_html (1)

zoo

as.yearqtr (1)

**NOTE:** Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately.

2. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has: - code in Python (1% in 1 files) and R (99% in 30 files) - 6 authors - 2 vignettes - 11 internal data files - 17 imported packages - 12 exported functions (median 37 lines of code) - 25 non-exported functions in R (median 44 lines of code) - 1 R function (median 21 lines of code) --- Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages The following terminology is used: - `loc` = "Lines of Code" - `fn` = "function" - `exp`/`not_exp` = exported / not exported All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by [the `checks_to_markdown()` function](https://docs.ropensci.org/pkgcheck/reference/checks_to_markdown.html) The final measure (`fn_call_network_size`) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile. |measure | value| percentile|noteworthy | |:------------------------|------:|----------:|:----------| |files_R | 31| 89.9| | |files_vignettes | 2| 85.7| | |files_tests | 13| 93.3| | |loc_R | 2065| 84.8| | |loc_vignettes | 378| 70.7| | |loc_tests | 225| 58.0| | |num_vignettes | 2| 89.2| | |data_size_total | 602894| 92.8| | |data_size_median | 1614| 67.2| | |n_fns_r | 37| 46.8| | |n_fns_r_exported | 12| 51.3| | |n_fns_r_not_exported | 25| 47.3| | |n_fns_src | 1| 0.0|TRUE | |n_fns_per_file_r | 1| 15.7| | |n_fns_per_file_src | 1| 0.1|TRUE | |num_params_per_fn | 4| 54.6| | |loc_per_fn_r | 41| 85.5| | |loc_per_fn_r_exp | 37| 70.2| | |loc_per_fn_r_not_exp | 44| 88.0| | |loc_per_fn_src | 21| 71.0| | |rel_whitespace_R | 17| 83.3| | |rel_whitespace_vignettes | 37| 75.3| | |rel_whitespace_tests | 29| 62.7| | |doclines_per_fn_exp | 25| 23.5| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 6| 24.8| | ---

2a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package

3. goodpractice and other checks

Details of goodpractice checks (click to open)

#### 3a. Continuous Integration Badges [![R-CMD-check](https://github.com/holatam/eph/workflows/R-CMD-check/badge.svg)](https://github.com/holatam/eph/actions) **GitHub Workflow Results** | id|name |conclusion |sha | run_number|date | |----------:|:--------------------------|:----------|:------|----------:|:----------| | 5382226608|pages build and deployment |success |707cd2 | 59|2023-06-26 | | 5382226669|pkgcheck |success |707cd2 | 3|2023-06-26 | | 5382226674|R-CMD-check |success |707cd2 | 903|2023-06-26 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) R CMD check generated the following note: 1. checking data for non-ASCII characters ... NOTE Note: found 1 marked Latin-1 string Note: found 1508 marked UTF-8 strings R CMD check generated the following check_fail: 1. rcmdcheck_non_ascii_characters_in_data #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 85.2 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) The following functions have cyclocomplexity >= 15: function | cyclocomplexity --- | --- calculate_tabulates | 32 get_microdata_internal | 26 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 1068 potential issues: message | number of times --- | --- Avoid library() and require() calls in packages | 19 Lines should not be more than 80 characters. | 1049

4. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following function name is duplicated in other packages: - - `get_microdata` from ech

Package Versions

|package |version | |:--------|:--------| |pkgstats |0.1.3.4 | |pkgcheck |0.1.1.26 |

Editor-in-Chief Instructions:

This package is in top shape and may be passed on to a handling editor

maelle commented 1 year ago

@ropensci-review-bot assign @maurolepore as editor

ropensci-review-bot commented 1 year ago

Assigned! @maurolepore is now the editor

maelle commented 1 year ago

@caropradier thanks for your submission!

maurolepore commented 1 year ago

Gracias @caropradier por compartir tu trabajo con nosotros. Es un gusto ser el editor.

Para ayudarte a rastrear mis comentarios, los etiqueto con "ml" y una secuencia numérica: ml01, ml02, etc:

Aca van mis primeros comentarios:

Estoy por comenzar mis chequeos iniciales^2. Pronto los voy a publicar aca.

^1 The potential suggestions made by the submitter(s), (although submitters may have a narrow view of the types of expertise needed. We suggest not using more than one of suggested reviewers) -- https://devguide.ropensci.org/editorguide.html

^2 After automatic checks are posted, use the editor template to guide initial checks and record your response to the submission -- https://devguide.ropensci.org/editorguide.html

caropradier commented 1 year ago

Gracias Mauro!

ml01. Confirmo que la revisión es en Castellano

ml02. Sugiero a:

  1. Karina Bartolomé
  2. Guadalupe Gonzalez
  3. Andrea Gomez Vargas
maurolepore commented 12 months ago

@caropradier,

Muchas gracias por tu paciencia. Aca comparto mis comentarios. Seria genial abordar los requerimientos antes de bucar revisores. Si tenes alguna duda no dudes en preguntarme.

(Perdon por los horrores de ortografia)

Editor checks:

Si.

Si.

Si. No veo detalles sobre tokens. Asumo que son innecesarios.

Si.

Si. Veo que las "issues" pendientes son pocas.

Editor comments

DOCUMENTACION
https://holatam.github.io/eph/
# Bien
# Comentario
x <- 1

# Mal
x <- 1  # Comentario

Esto ayuda a mantener los comentarios, porque el espacio disponble en una linea de comentario es totalmente independiente del especio disponible en una linea para codigo.

https://holatam.github.io/eph/articles/eph.html
authors:
  Mauro Lepore
    href: https://github.com/maurolepore
https://holatam.github.io/eph/articles/estimacion_pobreza.html
https://holatam.github.io/eph/news/index.html
TEMPLATE

¿Hay otros paquetes de R que logren el mismo objetivo? Si los hay, ¿cómo se diferencian del tuyo, o alcanzan nuestro criterio del mejor de su categoría (documento en Inglés)?

We strongly suggest submitting your package for review before publishing on CRAN or submitting a software paper describing the package to a journal. Review feedback may result in major improvements and updates to your package, including renaming and breaking changes to functions. We do not consider previous publication on CRAN or in other venues sufficient reason to not adopt reviewer or editor recommendations.

https://devguide.ropensci.org/policies.html#publishing-in-other-venues

ISSUES
CHECKS
CODE OVERVIEW
caropradier commented 11 months ago

Hola Mauro! ¿Cómo estás? Muchas gracias por tus comentarios y perdón por la demora. Estuve trabajando sobre estos comentarios y tengo algunas dudas/comentarios:

ml04. A los fines de cumplir con este requerimiento, agregué tests adicionales para la función interna is_in_github() para verificar el buen funcionamiento de los requests a la API de Github usando el paquete httptest. Sin embargo, no estoy segura de si sería necesario agregar tests a la función map_agglomerates() (que utiliza el paquete leaflet) para terminar de cumplir con esta condición; en este caso, tendrías algún consejo sobre qué herramientas puedo usar para implementarlos?

ml13. Los nombres de las columnas se mantienen con su formato original para imitar en la medida de lo posible las bases que se descargaría el usuario manualmente si no utilizara el paquete, y así preservar la compatibilidad con la documentación que publica INDEC.

ml15. ¿Estabas pensando en algún caso en particular? No encontré ninguna situación en la que esto pasara, únicamente se hace mucho énfasis en el nombre de los argumentos en las funciones que ilustran su funcionamiento (con fines expositivos).

ml19. Comento algunos ejemplos: https://cran.r-project.org/web/packages/ech/index.html https://cran.r-project.org/web/packages/PNADcIBGE/index.html https://cran.r-project.org/web/packages/readabs/index.html

ml20. Estoy al tanto. El objetivo es publicar una nueva versión en CRAN una vez que se complete el proceso de revisión

ml21. Quedan propuestas de funciones nuevas que creería que exceden el alcance del paquete pero pueden ser ideas interesantes para otro paquete, debería cerrar estos issues?

ml22. Todos los paquetes que aparecen listados como Imports se utilizan por lo menos una vez (se puede verificar con R-CMD check).

maurolepore commented 11 months ago

Gracias @caropradier,

ml04

No estoy segura de si sería necesario agregar tests a la función map_agglomerates(). ... Tendrías algún consejo sobre qué herramientas puedo usar para implementarlos?

Nuestra guia recomienda testear plots con vdiffr o snapshot tests.

For testing your functions creating plots, we suggest using vdiffr, an extension of the testthat package; or testthat snapshot tests. -- https://devguide.ropensci.org/building.html#testing

Sin embargo, los tests de aspectos visuales suelen ser sensibles a cambios insignificantes y por lo tanto dificiles de mantener. En la medida de lo posible, mi consejo es intentar testear el objecto detras del grafico. Podes asignar el plot a un objecto e investigar su estructura para ver que elementos pueden servirte para escribir los tests.

Ejemplo ``` r library(eph) library(testthat) data <- data.frame(AGLOMERADO = 2L, tasa_actividad = 0.2) p <- map_agglomerates(data, agglomerates = AGLOMERADO, indicator = tasa_actividad) #> Assuming "lon" and "lat" are longitude and latitude, respectively names(p) #> [1] "x" "width" "height" "sizingPolicy" #> [5] "dependencies" "elementId" "preRenderHook" "jsHooks" attributes(p$x) #> $names #> [1] "options" "calls" "limits" #> #> $leafletData #> AGLOMERADO indicator nombre_aglomerado lon lat #> 1 2 0.2 Gran La Plata -57.97302 -34.94983 test_that("el `AGLOMERADO` es el esperado", { actual <- attributes(p$x)$leafletData$AGLOMERADO expected <- data$AGLOMERADO expect_equal(actual, expected) }) #> Test passed 🎉 ``` Created on 2023-07-23 with [reprex v2.0.2](https://reprex.tidyverse.org)

ml13

:+1:

ml15

¿Estabas pensando en algún caso en particular? ... Unicamente se hace mucho énfasis en el nombre de los argumentos en las funciones que ilustran su funcionamiento (con fines expositivos).

Lo note en los ejemplos de este articulo: https://holatam.github.io/eph/articles/eph.html

OK :-)

ml19

Gracias! El comentario inicial ahora incluye tus ejemplos.

ml20

:+1:

ml21

Quizas podrias crear un repo para hospedar un borrador del proximo paquete y mover ahi las issues que sean relevantes? Asi podrias cerrarlas en eph para mantenerlo enfocado, y aun asi asegurarte que las buenas ideas no se van a perder. Las issues quedarian vinculadas automaticamente asi que podrias seguirles el rastro.

ml22

:+1:

Ademas:

Ejemplo ``` r library(eph) # Parece que `trimester` es requerido. Quizas no deberia tener default? # https://design.tidyverse.org/def-required.html args(get_microdata) #> function (year = 2018, trimester = NA, wave = NA, type = "individual", #> vars = "all", destfile = NA) #> NULL out <- get_microdata() #> Warning in get_microdata(): No se pudo descargar la base de year 2018,trimester NA, wave NA, type individual. #> Mensaje: (is.numeric(trimester) | is.numeric(wave)) is not TRUE # El resultado no es un error pero tampoco parece util dim(out) #> [1] 0 0 # Ademas el resultado no es estable: Arriba tengo 0 columnas y aca tengo 177 # https://design.tidyverse.org/out-type-stability.html dim(get_microdata(trimester = 1)) #> [1] 57951 177 dim(get_microdata(trimester = 2)) #> [1] 57835 177 ``` Created on 2023-07-23 with [reprex v2.0.2](https://reprex.tidyverse.org)
caropradier commented 11 months ago

Perfecto! Gracias :)

ml04: Genial! Mejoré los tests en base a esta idea, pienso que ahora deberían estar mejor

ml21: buena idea! Lo hago

ml25: buen punto, modifiqué los valores por default. Hice lo mismo para get_eahu() y get_total_urbano()

Saludos!

maurolepore commented 11 months ago

@caropradier,

Revisando los comentarios veo que los siguientes items requeridos aparecen como pendientes y no encuentro una respuesta especifica que se refiera a ellos. Sin embargo veo en el repo que algunos ya fueron resueltos, y quizas eso aplique a todos. Podrias por favor confirmar si ya resolviste todos o justificar si no?

Creo que si exploro profundamente el repo podria encontra la respuesta pero te agradeceria si recordas el estado y podes comentar aca. Creo que seria la forma mas rapida de completar esta etapa y pasar a buscar revisoras/es.

Los items a confirmar son , ml03, ml05, ml06, ml16.

caropradier commented 11 months ago

Hola Mauro! Perdón, no me di cuenta de que no te había avisado. Ya están resueltos.

ml03. El texto actual incluye la importancia y necesidad del paquete:

Si querés procesar datos de la Encuesta Permanente de Hogares - INDEC mediante el lenguaje de programación R, la librería eph tiene por objeto facilitar tu trabajo.

El paquete cumple un rol fundamental en la democratización de la posibilidad de procesar los datos publicados por INDEC y así obtener conclusiones independientes de aquellas publicadas en los informes elaborados por el organismo. Dado que la información de la EPH constituye una de las principales fuentes para el análisis de las problemáticas sociales presentes en Argentina, el paquete no sólo posibilita investigaciones académicas y periodísticas, sino que también contribuye a la formulación de políticas públicas fundamentadas en evidencia.

ml05. Se incluye el resultado

ml06. Efectivamente, apliqué usethis::use_tidy_style() al repositorio, y verifiqué que los cambios fueran acordes a las falencias que mencionabas

ml16. Efectivamente, actualmente se utiliza en todos los casos <-

maurolepore commented 11 months ago

@ropensci-review-bot assign @guadag12 as reviewer

ropensci-review-bot commented 11 months ago

@guadag12 added to the reviewers list. Review due date is 2023-08-27. Thanks @guadag12 for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

ropensci-review-bot commented 11 months ago

@guadag12: If you haven't done so, please fill this form for us to update our reviewers records.

maurolepore commented 10 months ago

@ropensci-review-bot assign @lidefi87 as reviewer

ropensci-review-bot commented 10 months ago

@lidefi87 added to the reviewers list. Review due date is 2023-09-09. Thanks @lidefi87 for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

ropensci-review-bot commented 10 months ago

@lidefi87: If you haven't done so, please fill this form for us to update our reviewers records.

maurolepore commented 10 months ago

@ropensci-review-bot set due date for @guadag12 to 2023-09-09

ropensci-review-bot commented 10 months ago

Review due date for @guadag12 is now 09-September-2023

ropensci-review-bot commented 9 months ago

:calendar: @lidefi87 you have 2 days left before the due date for your review (2023-09-09).

guadag12 commented 9 months ago

Hola, les dejo mi revisión.

Como pueden ver se centra en aspectos relacionados con la funcionalidad del paquete en cuestión y la calidad de su documentación😊. Espero que les sea útil.

Revisión de EPH

Por favor trata de marcar tantas casillas como te sea posible y elabora tus argumentos en comentarios abajo de cada una. Tu revisión no esta limitada a estos temas, tal como se describe en la guia para revisores (Reviewer Guide)

Por favor describe cualquier relación de trabajo que tengas/hayas tenido con los autores del paquete)

Antes que nada, felicitaciones por el paquete. Es un paquete sumamente útil para la comunidad y uno de los más importantes para aquellos que trabajan en política pública. Además de que es un ejemplo para otros organismos del estado de cómo se puede disponibilizar data que sea en servicio de la comunidad. Además, es una linda muestra de cómo se puede mejorar un proyecto con el tiempo y no sólo se sostiene en el mismo sino que está permanentemente creciendo.

En terminos generales funciona muy bien, corre rápido, es simple de usar y tiene mucha información. Ahora si, procedo a la review siguiendo las recomendaciones de rOpenSci:

Documentación

Acá me gustaría hacer una aclaración. Hay ejemplos de funciones que no me corrieron localmente cuando las corrí, lanzaron error. Me refiero a los ejemplos que figuran en los r files de github (por ejemplo acá)

Son las siguientes:

  1. get_total_urbano():
   base_individual <- get_total_urbano(
    year = 2016:2019,
    type = "individual",
    vars = c("AGLOMERADO", "PONDERA", "ESTADO", "CAT_OCUP")
    )

#Error in get_total_urbano(year = 2016:2019, type = "individual", vars = c("AGLOMERADO",  : could not find function "get_total_urbano"
  1. get_microdata():
 base_individual <- get_microdata(
   year = 2018:2019,period = 1,
   type = "individual",
   vars = c("PONDERA", "ESTADO", "CAT_OCUP")
    )
#Error in get_microdata(year = 2018:2019, period = 1, type = "individual",  :  unused argument (period = 1)
  1. get_eahu(): 
    base_individual <- get_eahu(
 year = 2010:2012,
 type = "individual",
  vars = c("SUBDOMINIO", "PONDERA", "ESTADO", "CAT_OCUP")
)
#Error in get_eahu(year = 2010:2012, type = "individual", vars = c("SUBDOMINIO",  :  could not find function "get_eahu"

Funcionalidad

En este punto hay dos funciones cuyo elapsed time es mayor a 5 segundos. Sin embargo, me parece que siguen corriendo en un tiempo prudencial. No es un aspecto que me parezca grave, simplemente si pueden chequearlo estaría genial, pero no me parece grave.

library(devtools)
mydir <- file.path (tempdir (), "srr-demo")
gert::git_clone ("https://github.com/holatam/eph", path = mydir)
devtools::check(mydir)

# checking examples (1m 53.3s)
# Examples with CPU (user + system) or elapsed time > 5s
# user system elapsed
# get_total_urbano 30.08   0.67   97.26
# get_eahu          4.11   0.22    9.33  

#0 errors v | 1 warning x | 0 notes v

library(goodpractice)
library(eph)
pkg_path <- system.file( package = "eph")
g <- gp(pkg_path)
g

Hice pruebas unitarias con testthat y hay algunas funciones que no tienen su test unitario en la carpeta de tests. Sin embargo, hay un 79% que si tienen. Como verán en el listado de abajo hay algunos que tal vez no es necesario que esten al 100% y hay otros que si. Depende mucho de cada función en particular. Les dejo el listado para que vea cuáles son y si creen necesario reforzar los test en algunas funciones por sobre otras, esta perfecto:

library(covr)
cov <- package_coverage(mydir)
report(cov)
print(cov)

#eph Coverage: 78.79%
#R/get_microdata.R: 0.00%
#R/get_microdata_internal.R: 0.00%
#R/get_poverty_lines.R: 0.00%
#R/is_in_github.R: 0.00%
#R/get_total_urbano_internal.R: 11.54%
#R/get_eahu_internal.R: 16.22%
#R/calculate_tabulates.R: 53.33%
#R/calculate_errors.R: 57.14%
#R/organize_caes.R: 74.07%
#R/calculate_poverty.R: 80.95%
#R/organize_panels.R: 84.62%
#R/get_eahu.R: 87.04%
#R/get_total_urbano.R: 87.04%
#R/organize_labels.R: 99.56%
#R/map_agglomerates.R: 100.00%
#R/organize_cno.R: 100.00%

Si, efectivamente sigue con la directrices de empaque de rOpenSci. Teniendo en cuenta, el nombre del paquete (corto, descriptivo, haciendo eco de su unicidad). En Windows al menos funciona bien. Los nombres de las funciones siguen el esquema object_verb() y es compatible su uso con Tidyverse y los nombres de las funciones no se confunden con las de otros paquetes como ggplot2, dplyr, magrittr, data.table. Ademas, usan message() y warning() siempre que es necesario. Además el uso de <- sobre el = es consistente en todas las funciones.

Estimación de horas dedicadas a la revisión: 5 horas

Comentarios de la revisión

Hay dos errores particulares que también me gustaría mencionar, pero que me parecen opcionales en su ejecución.

El primero es que muchas líneas de código superan los 80 caracteres. Es un error recurrente, pero si el código asi lo requiere creo que esta bien. Ademas, esta prolijo y se lee fácil. Es un error que salta cuando hago el chequeo con pkgcheck, pero tambien con goodpractice:

library(pkgcheck)
Sys.setenv ("GITHUB_TOKEN" = "<token>")
mydir <- file.path (tempdir (), "srr-demo")
gert::git_clone ("https://github.com/holatam/eph", path = mydir)
x <- pkgcheck (mydir)
print(x)

El segundo es que hay caracter Latin-1 en los .R files según goodpractice package.

pkg_path <- system.file( package = "eph")
g <- gp(pkg_path)
g

#fix this R CMD check NOTE: Note: found 1 marked Latin-1 string Note: found 1508 marked UTF-8 strings

Cuando intento averiguar qué archivo es, me figuran los siguientes (eliminando los .Rmd y .html por supuesto)...

library(readr)

# Function to search for Latin-1 strings in a file
search_encoding <- function(file_path) {
  # Guess the encoding of the file
  guessed_encoding <- readr::guess_encoding(file_path, n_max = 100)

  # Check if Latin-1 appears in the guessed encoding
  if ("ISO-8859-1" %in% guessed_encoding$encoding) {
    message(paste("Possible Latin-1 string in file:", file_path))
    print(guessed_encoding)
  }
}
# Directory of your package (change this path)
pkg_dir <- "C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo"
# List all files in the package directory recursively
all_files <- list.files(pkg_dir, pattern = "\\.(R|r|Rmd|Rnw|txt|csv)$", recursive = TRUE, full.names = TRUE)

# Search for Latin-1 strings in each file
for (file_path in all_files) {
  try({
    search_encoding(file_path)
  }, silent = TRUE)
}

# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/organize_panels.R
# PPossible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/diccionario_aglomerados.R
# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/canastas_reg_example.R
# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/calculate_errors.R
# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/doc/eph.R
# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/data-raw/caes.R
# ossible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/organize_cno.R
# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/errores_muestrales.R
# Possible Latin-1 string in file: C:/Users/User/AppData/Local/Temp/Rtmp2jNd9Y/srr-demo/R/diccionario_regiones.R

Por último, para cerrar, los quiero felicitar🎉. Es un gran trabajo el que ha hecho y nos simplifican mucho el trabajo a todos los que trabajamos con política pública.

De todo lo que mencioné tal vez le prestaría atención a dos aspectos:

  1. Los ejemplos de las funciones que mencionaba arriba. Creo que en alguna actualización quedaron desfasados y lo modificaría.
  2. Los test unitarios en las funciones que consideren más relevantes los agregaría. Insisto, tal vez no es necesario que todas las funciones tengan al 100% los test, pero a los que entiendan como más relevantes si lo agregaría.

Una vez que eso está, creo que estamos para avanzar 😊. Gran trabajo,

Saludos!

Todo esto lo corrí en mi computadora que tiene la siguiente versión: 

sessionInfo()

# R version 4.1.1 (2021-08-10)
# Platform: x86_64-w64-mingw32/x64 (64-bit)
# Running under: Windows 10 x64 (build 19045)
# 
# Matrix products: default
# 
# locale:
#   [1] LC_COLLATE=Spanish_Argentina.1252  LC_CTYPE=Spanish_Argentina.1252   
# [3] LC_MONETARY=Spanish_Argentina.1252 LC_NUMERIC=C                      
# [5] LC_TIME=Spanish_Argentina.1252    
# 
# attached base packages:
#   [1] stats     graphics  grDevices utils     datasets  methods   base     
# 
# other attached packages:
#   [1] forcats_1.0.0      ggplot2_3.4.1      lubridate_1.9.2    readr_2.1.4       
# [5] tidyr_1.3.0        dplyr_1.1.2        eph_0.6.1          pkgcheck_0.1.2.003
# 
# loaded via a namespace (and not attached):
#   [1] ghql_0.1.0               colorspace_2.1-0         selectr_0.4-2           
# [4] ellipsis_0.3.2           class_7.3-19             gitcreds_0.1.2          
# [7] leaflet_2.2.0            rprojroot_2.0.3          snakecase_0.11.0        
# [10] htmlTable_2.4.1          fs_1.5.2                 httpcode_0.3.0          
# [13] rstudioapi_0.15.0        proxy_0.4-27             farver_2.1.1            
# [16] roxygen2_7.2.3           urltools_1.7.3           remotes_2.4.2.1         
# [19] gh_1.4.0                 bit64_4.0.5              fansi_1.0.4             
# [22] xml2_1.3.3               splines_4.1.1            knitr_1.43.15           
# [25] jsonlite_1.8.4           whoami_1.3.0             maditr_0.8.3            
# [28] compiler_4.1.1           httr_1.4.7               backports_1.4.1         
# [31] Matrix_1.5-3             assertthat_0.2.1         fastmap_1.1.1           
# [34] lazyeval_0.2.2           cli_3.6.0                leaflet.providers_1.13.0
# [37] visNetwork_2.1.2         htmltools_0.5.5          prettyunits_1.1.1       
# [40] tools_4.1.1              igraph_1.4.2             gtable_0.3.1            
# [43] glue_1.6.2               rappdirs_0.3.3           Rcpp_1.0.10             
# [46] jquerylib_0.1.4          xopen_1.0.0              vctrs_0.6.1             
# [49] crul_1.4.0               nlme_3.1-152             crosstalk_1.2.0         
# [52] xfun_0.39                stringr_1.5.0            ps_1.7.2                
# [55] brio_1.1.3               rvest_1.0.3              timechange_0.2.0        
# [58] lifecycle_1.0.3          sys_3.4.1                goodpractice_1.0.4      
# [61] srr_0.0.1.194            scales_1.2.1             vroom_1.6.3             
# [64] clisymbols_1.2.0         hms_1.1.3                credentials_1.3.2       
# [67] rex_1.2.1                parallel_4.1.1           xmlparsedata_1.0.5      
# [70] RColorBrewer_1.1-3       httr2_0.2.2              gert_1.9.2              
# [73] yaml_2.3.7               curl_5.0.0               gridExtra_2.3           
# [76] lintr_3.1.0              rcmdcheck_1.4.0          triebeard_0.4.1         
# [79] stringi_1.7.12           desc_1.4.2               e1071_1.7-13            
# [82] checkmate_2.1.0          cyclocomp_1.1.1          pkgbuild_1.4.2          
# [85] attempt_0.3.1            matrixStats_0.63.0       rlang_1.1.0             
# [88] pkgconfig_2.0.3          lattice_0.20-44          evaluate_0.21           
# [91] purrr_1.0.1              sf_1.0-14                labeling_0.4.2          
# [94] htmlwidgets_1.6.1        bit_4.0.5                processx_3.8.0          
# [97] tidyselect_1.2.0         magrittr_2.0.3           R6_2.5.1                
# [100] expss_0.11.6             generics_0.1.3           DBI_1.1.3               
# [103] mgcv_1.8-36              pillar_1.9.0             withr_2.5.0             
# [106] units_0.8-1              tibble_3.2.1             praise_1.0.0            
# [109] janitor_2.2.0            crayon_1.5.2             KernSmooth_2.23-20      
# [112] utf8_1.2.3               tzdb_0.3.0               rmarkdown_2.24          
# [115] viridis_0.6.2            usethis_2.1.6            grid_4.1.1              
# [118] data.table_1.14.8        callr_3.7.3              graphql_1.5.1           
# [121] digest_0.6.31            classInt_0.4-9           covr_3.6.2              
# [124] openssl_2.0.6            munsell_0.5.0            viridisLite_0.4.1       
# [127] askpass_1.1              pkgstats_0.1.3.008
lidefi87 commented 9 months ago

Revisión del paquete eph

Por favor trata de marcar tantas casillas como te sea posible y elabora tus argumentos en comentarios abajo de cada una. Tu revisión no esta limitada a estos temas, tal como se describe en la guia para revisores (Reviewer Guide)

Por favor describe cualquier relación de trabajo que tengas/hayas tenido con los autores del paquete)

Documentación

El paquete incluye todos los siguiente tipos de documentación:

Funcionalidad

Estimación de horas dedicadas a la revisión: 5 horas

Comentarios de la revisión

Buen trabajo creando este paquete. De seguro será de gran utilidad para quienes quieran analizar este tipo de datos complejos. Les incluyo mis comentarios abajo.

Comentarios generales En general la documentación del paquete es clara, pero contiene varios acrónimos que no están definidos en el texto (por ej., INDEC, CNO, CAES, incluso EPH). Creo que valdría la pena definir estos acrónimos la primera vez que son referenciados en el texto. Esto es desde el punto de vista de alguien que no vive en la Argentina.

El sitio web relacionado a este paquete es bastante detallado y los ejemplos corren de manera local sin problema. La viñetas son en gran parte informativas, pero creo que ciertas funciones deberían incluir un poco más de contexto. Por ejemplo, sugeriría que la función organize_labels() incluya un poco más de información acerca del "etiquetado". Inicialmente pensé que esta función resultaría en nombres de columnas más descriptivas y no encontré un cambio al imprimir el nombre de las columnas en la consola usando (names(df)). No fue sino hasta que vi el set de datos en una pestaña aparte que entendí qué es lo que hacía esta función. Pero debo recalcar que no acostumbro usar las pestañas para ver datos porque suelo trabajar con sets de datos bastante grandes. En las viñetas, adulto_equivalente no incluye una definición de "adulto equivalente". Quizás esta definición sea obvia entre sociólogos, pero para alguien como yo que no formar parte de esta área no me queda claro a qué se refiere esto. Creo que al no proveer definiciones un poco más detalladas se podría estar limitando a las personas que puedan encontrar el paquete útil.

Problemas con ejemplos

  1. El ejemplo incluido bajo map_agglomerates me dio como resultado solo un mapa base. Al hacer una revisión más detallada, noté que el problema no era la base de datos toybase_individual_2016_04 ni los cálculos realizados sobre ella, sino más bien en la función map_agglomerates. Al revisar el código para esta función noté que si removía leaflet::addProviderTiles(leaflet::providers$Wikimedia) %>% de la línea 12, obtengo el resultado esperado: un mapa con círculos que representan el valor del indicador escogido. Adicionalmente, noté que la paleta de colores no permite cambios porque está codificada (hard coded) en la línea 8 (pal <- leaflet::colorNumeric(palette = "viridis", domain = df$indicator)). Al reemplazar palette = "viridis" con palette = palette la paleta de colores cambia en el mapa.

  2. El ejemplo incluido bajo organize_caes me dio la siguiente advertencia: 

Warning message:
In organize_caes(base = bases) : Convirtiendo PP04B_COD a character

No estoy segura de si este resultado es esperado o no.

Resultados de revisión automática

library(pkgreviewr)
pkgreview_create(pkg_repo = "holatam/eph", 
                 review_parent = "revision_eph")
devtools::check("revision_eph/eph")

── R CMD check results ────────────────────────────────────────────────────────────────────── eph 0.6.1 ────
Duration: 4m 18.5s

0 errors ✔ | 0 warnings ✔ | 0 notes ✔
   * checking re-building of vignette outputs ... [68s] OK
   * checking for non-standard things in the check directory ... OK
   * checking for detritus in the temp directory ... OK
   * DONE

   Status: OK

── R CMD check results ────────────────────────────────────────────────────────────────────── eph 0.6.1 ────
Duration: 4m 18.5s

0 errors ✔ | 0 warnings ✔ | 0 notes ✔
devtools::test("revision_eph/eph/")
ℹ Testing eph
ℹ Testing eph
✔ | F W S  OK | Context
✔ |   1     6 | calculate_errors [1.1s]            

══ Results ═════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 53.8 s

[ FAIL 0 | WARN 3 | SKIP 0 | PASS 57 ]
Warning message:
package ‘testthat’ was built under R version 4.2.3

Las advertencias fueron las siguientes: 

Warning (test-calculate_errors.R:11:3): checkear shape ds
There was 1 warning in `dplyr::mutate()`.
ℹ In argument: `ds = calculate_errors(...)`.
Caused by warning:
! Using an external vector in selections was deprecated in tidyselect 1.1.0.
ℹ Please use `all_of()` or `any_of()` instead.
  # Was:
  data %>% select(measure)

  # Now:
  data %>% select(all_of(measure))
....

Warning (test-get_eahu.R:12:3): selec variables
There was 1 warning in `dplyr::mutate()`.
ℹ In argument: `microdata = purrr::pmap(...)`.
Caused by warning:
! Using an external vector in selections was deprecated in tidyselect 1.1.0.
ℹ Please use `all_of()` or `any_of()` instead.
  # Was:
  data %>% select(vars)

  # Now:
  data %>% select(all_of(vars))
....

Warning (test-is_in_github.R:1:1): (code run outside of `test_that()`)
package ‘curl’ was built under R version 4.2.3
caropradier commented 9 months ago

Muchas gracias por todos sus comentarios! Apenas me sea posible voy a empezar a trabajar para incorporarlos :)

maurolepore commented 9 months ago

@guadag12, @lidefi87, muchísimas gracias por su trabajo detallado y puntual.

En el proximo comentario voy a registrar formalmente la entrega de sus revisiones.

Próximos pasos:

  1. @caropradier incorporará las revisiones y nos avisará cuando esté lista para su consideración.
  2. Yo les pediré que indiquen si los cambios satisfacen sus expectativas o si tienen más sugerencias.
maurolepore commented 9 months ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/593#issuecomment-1709968472 time 5

ropensci-review-bot commented 9 months ago

Logged review for guadag12 (hours: 5)

maurolepore commented 9 months ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/593#issuecomment-1712427212 time 5

ropensci-review-bot commented 9 months ago

Logged review for lidefi87 (hours: 5)

caropradier commented 9 months ago

@guadag12 gracias de nuevo por tu trabajo! Incorporé tus comentarios, incluyo algunas observaciones para seguir intercambiando en caso de que sea necesario:

  1. Respecto a los Ejemplos que no corren localmente. En los tres casos, los ejemplos no corren localmente porque se trata de funciones nuevas (o que fueron modificadas) desde el último submit a CRAN. Si se carga el paquete usando devtools::install_github("holatam/eph"), los ejemplos funcionan normalmente (al completar el proceso de revisión pienso enviar a CRAN la última versión, por lo que este conflicto se resolverá).

  2. Gracias por señalar la cuestión del elapsed time! Si no me equivoco, cuando estas dos funciones se incorporen a la versión en CRAN el problema dejará de ser de importancia (los tests incluyen skip_on_cran()).

  3. Respecto a los tests, obviamente estoy de acuerdo con la pertinencia de testear lo máximo posible. Simplemente, la función package_coverage() no tiene en cuenta los tests en algunos casos (al usar skip_if_xxxxx()). Por ejemplo, get_microdata() aparece con 0% a pesar de contar con tres tests (que se encuentran en test-get_microdata.R).

  4. Respecto a las líneas de código superan los 80 caracteres, coincido en que no es una buena práctica, únicamente sucede en scripts de input de datos, no en aquellos que describen las funciones, donde tenemos como prioridad que el código sea claro y fácil de leer.

  5. Respecto a los carácteres Latin-1, creo haber eliminado los casos donde esto ocurría, gracias por señalarlo!

@lidefi87 gracias de nuevo por tus esfuerzos! Incorporé también tus comentarios, incluyo algunas observaciones para seguir intercambiando en caso de que sea necesario:

  1. Respecto a los Ejemplos que no corren localmente, entiendo que aplica lo mismo que señalé anteriormente para la primera revisión.

  2. Respecto al nombre de las funciones, intentaría no hacer modificaciones mayores para no perturbar el flujo de trabajo de nuestros usuarios actuales (aproximadamente 30 mil personas usan el paquete y quisiera evitar generarles inconvenientes si no se trata de algo fundamental para el funcionamiento del paquete). No obstante, estoy de acuerdo con que se trata de una buena práctica, y lo tendré presente al incorporar nuevas funciones.

  3. Muchas gracias por tu observación respecto a los acrónimos y otros conceptos tales como adulto_equivalente, no lo había notado y es cierto que puede ser confuso para nuevos usuarios. Incorporé modificaciones en la documentación para mejorar este aspecto.

  4. Desarrollé mejor lo que sucede al aplicar organize_labels() en la viñeta y la documentación de la función, espero que ahora sea más claro!

  5. Gracias por identificar el problema en map_agglomerates()! No lo había notado y ahora su funcionamiento es el esperado.

  6. Respecto a organize_caes(), efectivamente ese es el comportamiento esperado. Simplemente se informa al usuario de esta transformación.

  7. Modifiqué la sintaxis depreciada de select() por select( all_of() / any_of() )

maurolepore commented 9 months ago

@caropradier, gracias por incorporar los cambios pronto.

--

@guadag12 y @lidefi87, por favor indiquen una de estas dos opciones:

  1. Recomiendan más cambios.
  2. Aprueban el paquete. En tal caso, por favor, expresen su aprobación formalmente utilizando la plantilla de aprobación de revisión.
lidefi87 commented 9 months ago

Respuesta de quien hizo la revisión

Aprobación final (posterior a la revisión)

Estimación de horas dedicadas a la revisión: 0.5 horas @caropradier gracias por hacer los cambios tan rápido. No tengo sugerencias adicionales, y creo que las que ofrecí ya han sido incluidas en la versión más reciente. Volví a correr las pruebas sugeridas en la revisión e incluyo los resultados abajo para su información. En cuanto al punto 2 sobre el nombre de las funciones. Estoy totalmente de acuerdo contigo. Creo que no valdría la pena modificarlas ahora, pero sí creo que sería bueno tomar en cuenta el standard de ROpenSci en funciones futuras. Nuevamente muy buen trabajo 😊 ``` devtools::test("revision_eph/eph/") ── R CMD check results ───────────────────────────────────────────────────────────────────── eph 0.6.1 ──── Duration: 3m 25.1s 0 errors ✔ | 0 warnings ✔ | 0 notes ✔ ``` ``` devtools::check("revision_eph/eph") ══ Results ════════════════════════════════════════════════════════════════════════════════════════════════ Duration: 40.6 s [ FAIL 0 | WARN 0 | SKIP 0 | PASS 57 ] ```
guadag12 commented 9 months ago

Respuesta de quien hizo la revisión

Aprobación final (posterior a la revisión)

Estimación de horas dedicadas a la revisión: 0

Gracias por tan pronta respuesta, @caropradier! Excelente los cambios. De mi parte no hay más modificaciones,

Me encantó el trabajo que han hecho, felicitaciones! 🎉

Saludos!

maurolepore commented 9 months ago

@ropensci-review-bot approve eph

ropensci-review-bot commented 9 months ago

Approved! Thanks @caropradier for submitting and @guadag12, @lidefi87 for your reviews! :grin:

To-dos:

Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent).

Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @ropensci/blog-editors in your reply. They will get in touch about timing and can answer any questions.

We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved.

Last but not least, you can volunteer as a reviewer via filling a short form.

maurolepore commented 9 months ago

@ropensci/blog-editors,

eph puede ser de gran interés:

cc: @yabellini

maurolepore commented 9 months ago

@caropradier,

Nota los To-dos y la informacion util publicada arriba. A medida que los hagas, por favor checkea los casilleros. En caso que no tengas privilegios para hacerlo, podes copiarlos en un nuevo comentario creado por vos misma:

- [ ] 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 will need to [enable two-factor authentication](https://docs.github.com/en/authentication/securing-your-account-with-two-factor-authentication-2fa/configuring-two-factor-authentication) for your GitHub account.
**This invitation will expire after one week. If it happens write a comment `@ropensci-review-bot invite me to ropensci/<package-name>` which will re-send an invitation.**
- [ ] After transfer write a comment `@ropensci-review-bot finalize transfer of <package-name>` where `<package-name>` is the repo/package name. This will give you admin access back.
- [ ] Fix all links to the GitHub repo to point to the repo under the ropensci organization.
- [ ] Delete your current code of conduct file if you had one since rOpenSci's default one will apply, see https://devguide.ropensci.org/collaboration.html#coc-file
- [ ] If you already had a `pkgdown` website **and are ok relying only on [rOpenSci central docs building and branding](https://devguide.ropensci.org/ci.html#even-more-ci-ropensci-docs)**,
    * deactivate the automatic deployment you might have set up
    * remove styling tweaks from your pkgdown config but keep that config file
    * replace the whole current `pkgdown` website with a [redirecting page](https://devguide.ropensci.org/redirect.html)
    * replace your package docs URL with `https://docs.ropensci.org/package_name`
    * In addition, in your DESCRIPTION file, include the docs link in the `URL` field alongside the link to the GitHub repository, e.g.: `URL: https://docs.ropensci.org/foobar, https://github.com/ropensci/foobar`
- [ ] Skim the docs of the [pkgdown automatic deployment](https://devguide.ropensci.org/building.html#docsropensci), in particular if your website needs MathJax.
- [ ] Fix any links in badges for CI and coverage to point to the new repository URL. 
- [ ] Increment the package version to reflect the changes you made during review. In NEWS.md, add a heading for the new version and one bullet for each user-facing change, and each developer-facing change that you think is relevant.
- [ ] We're starting to roll out software metadata files to all rOpenSci packages via the Codemeta initiative, see https://docs.ropensci.org/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.
- [ ] You can add this installation method to your package README `install.packages("<package-name>", repos = "https://ropensci.r-universe.dev")` thanks to [R-universe](https://ropensci.org/blog/2021/06/22/setup-runiverse/).

Ademas, nota nuestra guía sobre publicación de paquetes, marketing y preparación para GitHub.

yabellini commented 9 months ago

@ropensci/blog-editors,

eph puede ser de gran interés:

* Mucha gente ya usa este paquete.

* Según creo, es el primer paquete de rOpenSci que resultó del programa de campeonas y campeones.

* Fue revisado en Castellano.

cc: @yabellini

Sii exacto! por todos los puntos que mencionas!. Que emoción ver esto hecho una realidad. Felicitaciones a todes y muchas gracias todas las personas involucradas. Hablaré con Caro pero esto puede ser un artículo con todos los autores del paquete, no solo Caro, nuestra campeona.

Otra cosa mas, tenemos una etiqueta del programa de campeones para agregar en el repositorio del paquete. Compartiré los detalles para que la puedan agregar.

caropradier commented 9 months ago

@ropensci-review-bot finalize transfer of eph

ropensci-review-bot commented 9 months ago

Transfer completed. The eph team is now owner of the repository and the author has been invited to the team

caropradier commented 8 months ago

Hola! Perdón por la demora. Todo listo:

Por el momento dejaría la página de pkgdown si no es un problema, cuando tenga un poco más de tiempo puedo ocuparme de hacer esa migración :)