agroclimatico: Índices y Estadísticos Climáticos e Hidrológicos #599
Comments
|
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
|
🚀 Editor check started 👋 |
Checks for agromet (v1.0.0)git hash: d537dd20
(Checks marked with 👀 may be optionally addressed.) Package License: GPL-3 + file LICENSE 1. Package DependenciesDetails 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.
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(<path/to/repo>)', and examining the 'external_calls' table. baselist (38), c (23), data.frame (18), q (14), file (9), paste0 (8), args (7), dir (6), if (6), length (6), format (5), return (5), suppressWarnings (5), file.path (4), is.finite (4), names (4), seq_along (4), kappa (3), lapply (3), max (3), mean (3), normalizePath (3), source (3), sum (3), url (3), abs (2), as.data.frame (2), call (2), cumsum (2), diff (2), for (2), gamma (2), ifelse (2), is.na (2), log (2), match.call (2), mean.Date (2), min (2), missing (2), rep (2), system.file (2), unique (2), as.character (1), deparse (1), do.call (1), drop (1), exp (1), gsub (1), match.fun (1), mode (1), pmatch (1), range (1), readLines (1), rle (1), seq (1), signif (1), sink (1), strsplit (1), substitute (1), switch (1), tempdir (1) agrometcompletar_serie (5), glo.loglik (4), spi (4), dir_mapas (3), kringe (3), pdsi_coeficientes (3), C_pdsi (2), computar_olas (2), get_mapa (2), mapa_provincias (2), resolve_resolucion (2), agromet_informe (1), anomalia_porcentual (1), borrar_cache (1), compute_breaks (1), ConvertLongitude (1), coord_argentina (1), decil (1), descargar_mapa (1), dias_promedio (1), ith (1), kable_inta (1), lat_label (1), leer_nh (1), leer_surfer (1), lon_label (1), mapa_argentina (1), mapa_argentina_limitrofes (1), mapa_departamentos (1), mapear (1), metadatos_nh (1), olas (1), parglo.maxlik (1), pdsi (1), pdsi_ac (1), pdsi_internal (1), plot.metadatos_nh (1), replace_if_null (1), scale_color_inta (1), spi_params (1), surfer_to_hex (1) utilsdata (23), read.delim (4), capture.output (2), download.file (1), unzip (1) statsstart (10), ts (9), optim (3), end (2), ecdf (1), fitted (1) data.tableas.data.table (4), melt (4), month (4), year (2), rbindlist (1) grDevicescolours (5), palette (3), colorRampPalette (1), rgb (1) magrittr%>% (7) graphicspar (3), points (3) ggplot2geom_sf (2), aes (1), geom_point (1), scale_color_brewer (1) SPEIspi (5) clicli_abort (4) lmomcoare.parglo.valid (4) readrfwf_widths (2), read_fwf (2) gridrasterGrob (2), unit (1) automapautoKrige (2) sfread_sf (1), st_as_sf (1) pngreadPNG (1) rappdirsuser_data_dir (1) tidyrcomplete (1) NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately. 2. Statistical PropertiesThis 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:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
| id | name | conclusion | sha | run_number | date |
|---|---|---|---|---|---|
| 5708281034 | pkgcheck | success | d537dd | 13 | 2023-07-30 |
| 5708281033 | R-CMD-check | success | d537dd | 117 | 2023-07-30 |
| 5708281035 | test-coverage | success | d537dd | 54 | 2023-07-30 |
3b. goodpractice results
R CMD check with rcmdcheck
R CMD check generated the following check_fail:
- no_import_package_as_a_whole
Test coverage with covr
Package coverage: 96.73
Cyclocomplexity with cyclocomp
No functions have cyclocomplexity >= 15
Static code analyses with lintr
lintr found the following 183 potential issues:
| message | number of times |
|---|---|
| Avoid library() and require() calls in packages | 9 |
| Avoid trailing semicolons, they are not needed. | 1 |
| Lines should not be more than 80 characters. | 172 |
| Use <-, not =, for assignment. | 1 |
4. Other Checks
Details of other checks (click to open)
✖️ The following 3 function names are duplicated in other packages:
-
ithfrom bioclim
-
speifrom SPEI
-
spifrom bayestestR, precintcon, SPEI
Package Versions
| package | version |
|---|---|
| pkgstats | 0.1.3.4 |
| pkgcheck | 0.1.2.1 |
Editor-in-Chief Instructions:
This package is in top shape and may be passed on to a handling editor
|
@ropensci-review-bot assign @Pakillo as editor |
|
Assigned! @Pakillo is now the editor |
|
Gracias @paocorrales y colaboradoras por contribuir su paquete al proceso de revisión de rOpenSci, para el cual actuaré de editor invitado. Cualquier comentario o sugerencia que tengan durante el proceso, por favor no duden en comunicármelo. Algunos puntos que me gustaría tratar antes de comenzar el proceso de revisión: Alcance y solapamiento con otros paquetesEstoy de acuerdo en la amplia utilidad del paquete. Sólo un comentario: cuando se discute el posible solapamiento con otros paquetes se menciona
Igualmente, como sugerencia más que requisito, pienso que tal vez podría ser útil estudiar la compatibilidad con paquetes como ClimInd, que calcula más de 130 índices climáticos diferentes. Por ejemplo, mostrando algunos ejemplos de cómo integrar ambos paquetes para calcular distintos índices. Por último, veo que existe otro paquete llamado agrometR para descargar información de la Red Agroclimática Nacional de Chile (llamada agromet, https://www.agromet.cl/). ¿Piensan que la similitud en el nombre (agromet / agrometR) y tema (clima) de ambos paquetes podría ser problemático o confuso para futuros usuarios? En tal caso, ¿se plantearían un cambio de nombre del paquete? (ver guía rOpenSci). DependenciasEl chequeo automático sugiere que "Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately." Una búsqueda rápida en el código parecen indicar que los paquetes señalados (scales, ggnewscale, kableExtra, Rcpp) sí que se emplean, por lo que creo que podemos ignorar este comentario. Nombres de funcionesHay algunas funciones con nombre duplicado respecto a otros paquetes:
Me preocupa particularmente la duplicidad de nombres con el paquete SPEI (funciones DocumentaciónCreo que sería muy útil generar ya una web del paquete para facilitar el proceso de revisión. Esta web puede generarse automáticamente usando En el README, el tutorial (vignette) y algunas funciones se habla de datos en formato NH y estaciones NH. Estaría bien clarificar a qué se refiere NH. Por favor añadan unas directrices sobre cómo contribuir (ver https://devguide.ropensci.org/collaboration.html?q=contributi#contributing-guide). Chequeo del paqueteTras ejecutar Warning: [escalas.R:16] @Format requires a value Integración ContinuaPor favor modifiquen su GitHub Action para chequear el paquete en la versión actual de R, la anterior, y la que viene (ver guía). E idealmente tanto en Windows como Mac como Ubuntu. Para ello podría usarse esta plantilla generada por RevisoresPor favor sugieran el nombre de varios revisores potenciales del paquete. Muchas gracias |
|
Buenos días @paocorrales Sólo quería comprobar que recibieron mi mensaje anterior y saber si está todo claro. Si no es así, necesitan ayuda con algo, o simplemente necesitan más tiempo, por favor avísenme. En cuanto resolvamos estas cuestiones preliminares podremos empezar la revisión con revisores externos Gracias y saludos |
|
Muchas gracias por los comentarios y recomendaciones @Pakillo! Me disculpo por la demora, estaba de viaje pero esta semana voy a revisar todo en detalle y hacer los cambios necesarios para comenzar con la revisión. Saludos! |
|
¡Estupendo, gracias! |
|
Muchas gracias por los comentarios @Pakillo y perdón por la demora, respondo a cada punto entre lineas.
Muchas gracias por mencionar esto, si bien aún no está en CRAN voy a editar mi primer mensaje incluyendo el link al repositorio para que no genere confunción.
Muchas gracias por la sugerencia! Estuvimos explorando este paquete y si bien incluye muchas funciones en varios casos vimos que las funciones son algo repetitivas (por ejemplo muchas funciones para calcular la temperatura mayor a distintos umbrales, en vez de 1 función que haga eso para cualquier umbral arbitrario). Por otro lado, la salida de estas funciones no es siempre compatible con la filosofía tidy que usamos en nuestras funciones. De todas maneras lo vamos a seguir explorando!
Luego de consultar con el equipo decidimos cambiar el nombre del paquete a agroclimatico. No encontramos otros paquetes de R o software con este nombre y esperamos que genere menos confución. Quedo atenta por si hay que hacer cambios en este issue.
Entendemos la sugerencia, spi es una función presente en varios paquetes y además es el nombre que se usa para hacer referencia al índice aún en la literatura en español. Cambiamos el nombre de la función a spi_indice que si bien es algo redundante puee ayudar a evitar la confución.
Gracias por la sugerencia, el paquete ahora tiene una web creada con pkgdown: https://agrometeorologiainta.github.io/agroclimatico/
Agregamos algunas aclaraciones.
Las directrices sobre como contribuir están disponibles en https://github.com/AgRoMeteorologiaINTA/agromet/blob/master/.github/CONTRIBUTING.md y linkeadas al README
Warnings corregidos, gracias!
Actualizamos la GitHub Action para chequear el paquete con distintas versiones de R y sistemas operativos. En el Caso de Mac tuvimos que skipear los test visuales porque fallan debido a una incompatibilidad entre sistemas operativos que no debería afectar a le usuarie.
Sugerimos a Ana Laura Dietrich, ella tiene experiencia en el desarrollo de paquetes y en el área agrocliomática. |
|
@ropensci-review-bot check package |
|
Thanks, about to send the query. |
|
🚀 Editor check started 👋 |
|
@ropensci-review-bot help |
|
Hello @Pakillo, here are the things you can ask me to do: |
Checks for agroclimatico (v1.0.0)git hash: 2d86dbe8
(Checks marked with 👀 may be optionally addressed.) Package License: GPL-3 + file LICENSE 1. Package DependenciesDetails 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.
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(<path/to/repo>)', and examining the 'external_calls' table. baselist (38), c (23), data.frame (18), q (14), file (9), paste0 (8), args (7), dir (6), if (6), length (6), format (5), return (5), suppressWarnings (5), file.path (4), is.finite (4), names (4), seq_along (4), kappa (3), lapply (3), max (3), mean (3), normalizePath (3), source (3), sum (3), url (3), abs (2), as.data.frame (2), call (2), cumsum (2), diff (2), for (2), gamma (2), ifelse (2), is.na (2), log (2), match.call (2), mean.Date (2), min (2), missing (2), rep (2), system.file (2), unique (2), as.character (1), deparse (1), do.call (1), drop (1), exp (1), gsub (1), match.fun (1), mode (1), pmatch (1), range (1), readLines (1), rle (1), seq (1), signif (1), sink (1), strsplit (1), substitute (1), switch (1), tempdir (1) agroclimaticocompletar_serie (5), glo.loglik (4), dir_mapas (3), kringe (3), pdsi_coeficientes (3), C_pdsi (2), computar_olas (2), get_mapa (2), mapa_provincias (2), resolve_resolucion (2), agromet_informe (1), anomalia_porcentual (1), borrar_cache (1), compute_breaks (1), ConvertLongitude (1), coord_argentina (1), decil (1), descargar_mapa (1), dias_promedio (1), ith (1), kable_inta (1), lat_label (1), leer_nh (1), leer_surfer (1), lon_label (1), mapa_argentina (1), mapa_argentina_limitrofes (1), mapa_departamentos (1), mapear (1), metadatos_nh (1), olas (1), parglo.maxlik (1), pdsi (1), pdsi_ac (1), pdsi_internal (1), plot.metadatos_nh (1), replace_if_null (1), scale_color_inta (1), spi_indice (1), spi_params (1), surfer_to_hex (1) utilsdata (23), read.delim (4), capture.output (2), download.file (1), unzip (1) statsstart (10), ts (9), optim (3), end (2), ecdf (1), fitted (1) data.tableas.data.table (4), melt (4), month (4), year (2), rbindlist (1) grDevicescolours (5), palette (3), colorRampPalette (1), rgb (1) magrittr%>% (7) graphicspar (3), points (3) ggplot2geom_sf (2), aes (1), geom_point (1), scale_color_brewer (1) SPEIspi (5) clicli_abort (4) lmomcoare.parglo.valid (4) readrfwf_widths (2), read_fwf (2) gridrasterGrob (2), unit (1) automapautoKrige (2) sfread_sf (1), st_as_sf (1) pngreadPNG (1) rappdirsuser_data_dir (1) tidyrcomplete (1) NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately. 2. Statistical PropertiesThis 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:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
| id | name | conclusion | sha | run_number | date |
|---|---|---|---|---|---|
| 7095108109 | pages build and deployment | success | f43114 | 10 | 2023-12-05 |
| 7095086203 | pkgcheck | success | 2d86db | 21 | 2023-12-05 |
| 7095086206 | pkgdown | success | 2d86db | 8 | 2023-12-05 |
| 7095086213 | R-CMD-check | success | 2d86db | 125 | 2023-12-05 |
| 7095086208 | test-coverage | success | 2d86db | 62 | 2023-12-05 |
3b. goodpractice results
R CMD check with rcmdcheck
R CMD check generated the following check_fail:
- no_import_package_as_a_whole
Test coverage with covr
Package coverage: 96.73
Cyclocomplexity with cyclocomp
No functions have cyclocomplexity >= 15
Static code analyses with lintr
lintr found the following 190 potential issues:
| message | number of times |
|---|---|
| Avoid library() and require() calls in packages | 9 |
| Avoid trailing semicolons, they are not needed. | 1 |
| Lines should not be more than 80 characters. | 179 |
| Use <-, not =, for assignment. | 1 |
4. Other Checks
Details of other checks (click to open)
✖️ The following function name is duplicated in other packages:
-
ithfrom bioclim
Package Versions
| package | version |
|---|---|
| pkgstats | 0.1.3.9 |
| pkgcheck | 0.1.2.11 |
Editor-in-Chief Instructions:
This package is in top shape and may be passed on to a handling editor
|
Muchas gracias por la respuesta y todas las mejoras al paquete @paocorrales et al. Yo creo que está casi listo para pasar a revisión. Solo tres pequeños detalles por ahora:
Procedo a continuación a buscar revisores. Si pueden arreglar estos pequeños detalles mientras tanto, estupendo. |
Editor checks:
Editor comments¿Algunos issues parecen obsoletos y se podrían cerrar? Sería bueno cerrar lo que ya esté resuelto para indicar claramente a los revisores y usuarios/as del paquete el estado actual del paquete y cuáles son los temas abiertos. ¡Gracias! |
|
@ropensci-review-bot seeking reviewers |
|
Please add this badge to the README of your package repository: [](https://github.com/ropensci/software-review/issues/599)Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news |
Pakillo
changed the title
|
Muchas gracias @Pakillo, resolví todos los detalles mencionados 😃 |
|
Estupendo, muchas gracias @paocorrales |
|
@ropensci-review-bot check package |
|
Thanks, about to send the query. |
|
🚀 Editor check started 👋 |
Checks for agroclimatico (v1.0.0)git hash: a4b630c0
(Checks marked with 👀 may be optionally addressed.) Package License: GPL-3 + file LICENSE 1. Package DependenciesDetails 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.
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(<path/to/repo>)', and examining the 'external_calls' table. baselist (38), c (23), data.frame (18), q (14), file (9), paste0 (8), args (7), dir (6), if (6), length (6), format (5), return (5), suppressWarnings (5), file.path (4), is.finite (4), names (4), seq_along (4), kappa (3), lapply (3), max (3), mean (3), normalizePath (3), source (3), sum (3), url (3), abs (2), as.data.frame (2), call (2), cumsum (2), diff (2), for (2), gamma (2), ifelse (2), is.na (2), log (2), match.call (2), mean.Date (2), min (2), missing (2), rep (2), system.file (2), unique (2), as.character (1), deparse (1), do.call (1), drop (1), exp (1), gsub (1), match.fun (1), mode (1), pmatch (1), range (1), readLines (1), rle (1), seq (1), signif (1), sink (1), strsplit (1), substitute (1), switch (1), tempdir (1) agroclimaticocompletar_serie (5), glo.loglik (4), dir_mapas (3), kringe (3), pdsi_coeficientes (3), C_pdsi (2), computar_olas (2), get_mapa (2), mapa_provincias (2), resolve_resolucion (2), agromet_informe (1), anomalia_porcentual (1), borrar_cache (1), compute_breaks (1), ConvertLongitude (1), coord_argentina (1), decil (1), descargar_mapa (1), dias_promedio (1), ith (1), kable_inta (1), lat_label (1), leer_nh (1), leer_surfer (1), lon_label (1), mapa_argentina (1), mapa_argentina_limitrofes (1), mapa_departamentos (1), mapear (1), metadatos_nh (1), olas (1), parglo.maxlik (1), pdsi (1), pdsi_ac (1), pdsi_internal (1), plot.metadatos_nh (1), replace_if_null (1), scale_color_inta (1), spi_indice (1), spi_params (1), surfer_to_hex (1) utilsdata (23), read.delim (4), capture.output (2), download.file (1), unzip (1) statsstart (10), ts (9), optim (3), end (2), ecdf (1), fitted (1) data.tableas.data.table (4), melt (4), month (4), year (2), rbindlist (1) grDevicescolours (5), palette (3), colorRampPalette (1), rgb (1) magrittr%>% (7) graphicspar (3), points (3) ggplot2geom_sf (2), aes (1), geom_point (1), scale_color_brewer (1) SPEIspi (5) clicli_abort (4) lmomcoare.parglo.valid (4) readrfwf_widths (2), read_fwf (2) gridrasterGrob (2), unit (1) automapautoKrige (2) sfread_sf (1), st_as_sf (1) pngreadPNG (1) rappdirsuser_data_dir (1) tidyrcomplete (1) NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately. 2. Statistical PropertiesThis 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:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
| id | name | conclusion | sha | run_number | date |
|---|---|---|---|---|---|
| 7262878850 | pages build and deployment | success | 3df214 | 12 | 2023-12-19 |
| 7262805827 | pkgcheck | success | a4b630 | 23 | 2023-12-19 |
| 7262805825 | pkgdown | success | a4b630 | 10 | 2023-12-19 |
| 7262805832 | R-CMD-check | success | a4b630 | 127 | 2023-12-19 |
| 7262805830 | test-coverage | success | a4b630 | 64 | 2023-12-19 |
3b. goodpractice results
R CMD check with rcmdcheck
R CMD check generated the following check_fail:
- no_import_package_as_a_whole
Test coverage with covr
Package coverage: 96.73
Cyclocomplexity with cyclocomp
No functions have cyclocomplexity >= 15
Static code analyses with lintr
lintr found the following 189 potential issues:
| message | number of times |
|---|---|
| Avoid library() and require() calls in packages | 9 |
| Lines should not be more than 80 characters. | 179 |
| Use <-, not =, for assignment. | 1 |
4. Other Checks
Details of other checks (click to open)
✖️ The following function name is duplicated in other packages:
-
ithfrom bioclim
Package Versions
| package | version |
|---|---|
| pkgstats | 0.1.3.9 |
| pkgcheck | 0.1.2.11 |
Editor-in-Chief Instructions:
This package is in top shape and may be passed on to a handling editor
|
@ropensci-review-bot assign @VeruGHub as reviewer |
|
@VeruGHub added to the reviewers list. Review due date is 2024-01-09. Thanks @VeruGHub 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. |
|
📆 @VeruGHub you have 2 days left before the due date for your review (2024-01-09). |
|
@ropensci-review-bot assign @pmnatural as reviewer |
|
@pmnatural added to the reviewers list. Review due date is 2024-02-06. Thanks @pmnatural 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
added
3/reviewer(s)-assigned
and removed
2/seeking-reviewer(s)
labels
|
@pmnatural: If you haven't done so, please fill this form for us to update our reviewers records. |
|
Hola @paocorrales. Disculpas por la demora, ha costado bastante encontrar un segundo revisor. Ya está todo en marcha. @VeruGHub ¿cómo va la revisión? Espero que pueda estar lista pronto, para que los autores puedan empezar a revisar las sugerencias. Si puedo ayudar en algo no dude en avisarme. ¡Gracias! |
|
@Pakillo estoy con la revisión hoy. Disculpad la demora |
|
Estupendo. Gracias @VeruGHub. ¿Crees que podrás enviar la revisión esta semana? |
|
📆 @pmnatural you have 2 days left before the due date for your review (2024-02-06). |
|
Con algunos problemas al tener que cambiar de equipo. Estoy rehaciendo la revision en una maquina distinta y con mala internet. Espero subir el reporte mas tarde. |
Revisión del paquete AgroclimaticoPor favor describe cualquier relación de trabajo que tengas/hayas tenido con los autores del paquete) No he tenido ninguna relacion de trabajo con los autores del paquete.
DocumentaciónEl paquete incluye todos los siguiente tipos de documentación:
Funcionalidad
Estimación de horas dedicadas a la revisión:
Comentarios de la revisiónLos items sin tildar no han sido objeto de esta revision Seria util que despues de la descripcion inicial del paquete haya una lista de todas las funciones disponibles a la fecha de la version. Segun la viñeta estas serian: En la viñeta estan: En los man pages se suman estas ; Segun la descripcion del paquete, se asume que es solo para personal del INTA aunque otros usuarios externos podrian tambien beneficiarse. Para ello seria importante, si lo consideran conveniente los autores, incluir un breve comentarios sobre el tipo de archivo, estructura de datos y nombres de columnas que deberian tener estos datos. |
|
¡ Muchas gracias por su revisión @pmnatural ! Antes de continuar, ¿podría indicar una estimación del tiempo (horas) dedicado a la revisión? Para efectos puramente estadísticos del programa de revisión de rOpenSci Gracias |
|
Gracias Priscilla por la revisión! Respecto a tu comentario te consulto si la propuesta sería incorporar toda la lista de funciones en el README. Ahora no son muchas funciones y se podría hacer pero con suerte en el futuro espero sumar más y creo que quedaría muy largo. Una alternativa que se me ocurre es linkear desde el README al índice de la web del paquete donde está la lista de funciones: https://agrometeorologiainta.github.io/agroclimatico/reference/index.html Te parece una alternativa posible? |
Revisión de un paqueteNo tengo ninguna relación de trabajo con los autores
DocumentaciónEl paquete incluye todos los siguiente tipos de documentación:
Funcionalidad
Estimación de horas dedicadas a la revisión: 8
Comentarios de la revisiónEn primer lugar, quiero agradecer la oportunidad de revisar este paquete y espero que los comentarios sirvan para mejorar en los puntos que los autores consideren oportunos. Creo que los autores han hecho un gran trabajo compilando las funciones relacionadas con la gestión de datos climáticos que son útiles para el INTA. He revisado el paquete desde el punto de vista de un usuario y tengo algunos comentarios generales y también específicos de cada función. Incluyo en la revisión código que he ido ejecutando y con comentarios en partes que no han funcionado correctamente y reflexiones. Creo que algunos aspectos formales de la documentación pueden ser mejorados. Para empezar, la funcionalidad del paquete no está completamente definida en la documentación (Readme). El paquete calcula índices y estadísticos climáticos e hidrológicos a partir de datos tidy, pero también permite representar esta información y otra en mapas en Argentina, y permite leer datos climáticos en el formato del INTA. Creo que toda esta información debería de estar incluida en el Readme. En varios puntos de la documentación de las funciones (ver debajo) he detectado que no se especifica el formato concreto en el que los argumentos deben declararse y también argumentos cuya descripción es confusa. Mejorar estas descripciones e incluir programación defensiva relativa a los argumentos (en general faltante) ayudaría mucho a los usuarios. Creo que la funcionalidad de spi_indice y spei_indice está ya contenida en el paquete SPEI. A menos que se aclare cual es la novedad de las funciones escritas para agroclimático, sugiero eliminar estas funciones. Además, no he conseguido correr las funciones scale_color/fill_inta y pdsi/pdsi_ac (ver debajo los comentarios). Finalmente, en el logotipo del paquete pone "AgroMET" lo que es un poco raro si el paquete se llama "agroclimatico". dias_promedioNo calcula ningun estadístico meteorológico como dice la descripción si no que extrae dia, mes y dia juliano de un set de datos. Previamente el usuario puede haber extraido los días que representan un evento meteorológico de interés, pero no tiene por qué. La descripción de la funcionalidad no es clara en la ayuda. Sugiero poner "Calcula el valor promedio anual del primer y ultimo dia para una serie de fechas dada". En los ejemplos habría que cambiar "summarise" por "reframe" como aparece en el Readme. En "Value" se dice que el output tiene "variables extras en el caso de hacer el cálculo para distintos grupos", pero ¿cómo se puede especificar para qué grupos quieres el cálculo? Erratas en la documentación: "seria de fechas" completar_serieNo se especifica el formato del vector fecha ni de rango en la documentación. decil y anomalia_porcentualEn la descripción se necesita incluir una definición más técnica de qué valores se devuelven, es decir, que hace la función decil y la anomalia_porcentual y referencias. En estadística, los deciles de una serie de datos son estadísticos descriptivos que representan los valores que limitan la serie de datos en 10 partes. Al aplicar la función sin tener mucho detalle en la descripción yo hubiera esperado obtener eso. Sin embargo, en el ejemplo ejecutado debajo obtengo un vector de igual longitud de la serie de datos. Sugiero mejorar la descripción en este sentido. La anomalía porcentual ¿respecto a qué valor de la serie de referencia se calcula? Debería especificarse. ¿Por qué para calcular la anomalia es necesario especificar que no se consideren los NA, pero para decil no? Parece una cuestión de inconsistencia en cómo están escritas las funciones. scala_temp/pp_XXXLa documentación está incompleta. No hay ejemplos de uso ni se entiende bien que hacen estas funciones. Tampoco se especifica qué valores retorna. ithFunciona correctamente y está bien documentada. metadatos_nhPara facilitar la comprensión a los usuarios convendría especificar que son "estaciones NH". La descripción es incompleta en el sentido de que sólo se especifica una parte de la información que devuelve la función. Erratas en la documentación: "provincia o provincia", "especificar" debería ser "específicas". kable_intaSustituir "Setear" por "Formatear" o similar. Funciona correctamente. leer_surferEn el ejemplo se sobreescribe un objeto con el nombre de otra función implementada en el paquete (scala_temp/pp_XXX). Debería estar explicado en algún sitio cual es el link entre ambas. Erratas en la documentación: "laeer". mapa_XXXAñadiría en la descripción los posibles valores para el argumento "provincias". mapearErratas en la documentación: "veriable", "a el", "masyores", "1500m". El argumento "breaks" no está bien definido. Habría que especificar que se refiere a que hay que introducir valores (¿uno o varios?) en el rango del argumento "valor". No se explican las funciones coord_argentina ni theme_inta_mapa en ningún sitio. ¿Se utilizan sólo internamente? En ese caso no deberían estar exportadas. olas y umbralesNo se explica en la documentación como hay que especificar los umbrales. Es poco habitual que se pueda poner cualquier palabra como nombre de un argumento y una mala praxis porque se puede sobreescribir algo importante en la función. Sería más adecuado que hubiera un argumento para los umbrales y que éstos se pudieran introducir en forma de lista con nombres (ej. umbral = list(calor = "t_max > 20")) Corregir algunas erratas en la documentación de olas: "mbral", "drtn" Corregir algunas erratas en la documentación de umbrales: " pdsi/pdsi_acNo funciona indicandole los argumentos no definidos por defecto. Habría que incluir en la documentación las referencias que se citan e incluir las unidades de la precipitacion y la etp. Es redundante incluir un argumento para los coeficientes en forma de lista y luego argumentos para cada coeficiente por separado. Sugiero eliminar una de las dos formas. Corregir algunas erratas de la ayuda: "list de coeficientes", "El el cálculo". scale_color/fill_inta pdsi/pdsi_acEl ejemplo no funciona. Tampoco funciona otro ejemplo que he creado siguiendo las instrucciones de la documentación. Corregir erratas en la documentación: "escala.)", "tiene prioridad por sobre", "deinidos" spei_indice/spi_indiceLa funcionalidad está repetida respecto a las funciones spi y spei del paquete SPEI. A menos que se aclare qué aporta esta función sugiero eliminarla y crear una viñeta que incluya cómo se usa el paquete SPEI con datos en el formato del INTA. Si he entendido bien, las fechas deben introducirse como meses ¿no? Por favor, especificar esto si se mantiene la función. En la documentación no se entiende bien que es el argumento "escalas" y no se cita el paquete SPEI. |
|
¡Muchas gracias @VeruGHub por su detallada revisión! Seguro que será muy útil. |
|
@ropensci-review-bot submit review #599 (comment) time 8 |
|
Logged review for VeruGHub (hours: 8) |
|
@ropensci-review-bot submit review #599 (comment) |
|
I'm sorry human, I don't understand that. You can see what commands I support by typing:
|
|
@ropensci-review-bot submit review #599 (comment) time NA |
|
Logged review for pmnatural (hours: NA) |
ropensci-review-bot
added
4/review(s)-in-awaiting-changes
and removed
3/reviewer(s)-assigned
labels
|
Estimada @paocorrales y coautores: Ya tenemos las revisiones de @pmnatural y @VeruGHub, a quienes agradezco su esfuerzo. Espero que sus comentarios sean útiles. Añado algunos comentarios al respecto: READMEAmbas revisoras comentan que les gustaría ver más información sobre el paquete ahí. Yo la veo correcta (sobre todo al contar con un magnífico tutorial complementario (vignette)), pero tal vez se pueda añadir algo más o re-estructurar la información del README para que quede más claro, al menos en cuanto a los grandes bloques de funcionalidad del paquete (lectura de datos INTA, indicadores, mapeo, etc). Una sugerencia para que quede más clara la funcionalidad global del paquete es agrupar las funciones en el índice por temáticas (ver https://r-pkgs.org/website.html#index-organization). En el primer párrafo del Readme hay una errata ( En el segundo párrafo del Readme, la función En el tercer párrafo se menciona por primera vez el INTA. Es posible que muchos no conozcan a qué se refiere el acrónimo, por lo que añadiría el nombre completo del instituto y un enlace a la web oficial. Es cierto que el logo refleja el antiguo nombre del paquete (agromet), ¿tal vez sea buena idea modificarlo? La 'vignette' a mí si me funciona, siempre que se especifique dias_promedioEn esta y otras funciones, como indica @VeruGHub, sería conveniente reemplazar decilEn la función decil, pienso que sería útil dar la opción de devolver tanto el número del decil (1, 2, 3, ...) como el valor numérico (ej. 7.7 correspondente al decil 1, etc). Por ejemplo viendo el ejemplo en https://agrometeorologiainta.github.io/agroclimatico/articles/estadisticas-e-indices-climaticos.html#deciles no me queda claro cómo interpretar en qué decil nos encontramos simplemente viendo el valor de la columna 'decil'. mapearEsta función me parece muy útil pero me pregunto si podría rediseñarse para que el primer argumento sea un data.frame o tibble, siguiendo la costumbre en tidyverse. Esto permitiría llamarla directamente dentro de un pipe, p. ej.: datos_aleatorios |>
mapear(pp, lon, lat,
cordillera = TRUE,
escala = escala_pp_diaria,
variable = "mm",
titulo = "Precipitación aleatoria",
fuente = "Fuente: datos de ejemplo")olas y umbralesEstas funciones también me gustan mucho y me parecen muy útiles, pero igualmente me pregunto (a modo de reflexión) si el primer argumento podría ser un data.frame o tibble, dado que se usan varias variables del mismo para calcular las olas. Esto podría permitir además que no fuese necesario usar summarise/reframe, p. ej: olas(NH0358, calor = t_max > 20, frio = t_min <= 0)SPI, SPEITambién aquí me planteo si no sería conveniente (siguiendo la filosofía tidyverse) que el primer argumento fuera siempre un data.frame o tibble. De ese modo no habría que usar Estoy de acuerdo con @VeruGHub en que sería bueno explicar la utilidad de estas funciones y en qué difiere esta implementación de la del paquete SPEI (utilizado internamente). GeneralEn los ejemplos sería conveniente simplificar los datasets al máximo, dejando solamente lo necesario para mostrar lo que hace la función. Por ejemplo, en la función ErratasHay algunas erratas en la documentación. Podría usarse el paquete spelling o herramientas similares para facilitar su detección y corrección. Nada más por ahora. Espero que mis comentarios, unidos a los de las revisoras, sean útiles para revisar el paquete. Por favor no duden en comentar cualquier duda o discrepancia que puedan tener. Muchos de los cambios propuestos son sugerencias o invitaciones a la reflexión. Quedo pues a la espera de su revisión. Saludos cordiales Paco |
|
@paocorrales, @eliocamp, @yabellini, @NatiGattinoni: please post your response with Here's the author guide for response. https://devguide.ropensci.org/authors-guide.html |
Nombre de la Persona Encargada: Paola Corrales
Usuario GitHub de la Persona Encargada: @paocorrales
Usuario GitHub de las Otras Autoras del Paquete: @eliocamp, @yabellini, @NatiGattinoni
Repositorio: https://github.com/AgRoMeteorologiaINTA/agroclimatico
Versión Enviada: 1.0.0
Tipo de Envio: Estándar
Editora: @Pakillo
Revisores: @VeruGHub, @pmnatural
Archivo: TBD
Versión Aceptada: TBD
Idioma: es
Alcance
Por favor, indica qué categoría(s) aplican a este paquete. Las puedes encontrar en nuestras políticas de inclusión de paquetes (Inglés). Por favor, tilda todas las apropiadas. Si no estás seguro, te sugerimos que comiences un pre-envío.
Explica cómo y por qué el paquete encaja dentro de estas categorías (1 a 3 oraciones):
El paquete agromet incluye una serie de funciones para trabajar con datos meteorológicos y calcular distintos índices asociados a aplicaciones agrometeorológicas. Los datos de entrada se organizan usando la filosofía de datos ordenados (o tidy), por lo que las funciones del paquete son genéricas. Pueden aplicarse a cualquier conjunto de datos tabulares independientemente de su origen, orden o nombres de columna.
¿Cuál es la audiencia esperada y las aplicaciones científicas de este paquete?
El paquete es ampliamente utilizado por usuaries en distintas dependencias del Instituto Nacional de Tecnología Agropecuaria (INTA) que brindan servicios e información a los agricultores de Argentina. Sin embargo, les usuaries del paquete se extienden a otras instituciones y países de América Latina ya que casi todas las funciones están desarrolladas para ser de uso general e independiente de los datos recolectados por el INTA y la documentación está escrita integramente en español.
¿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)?
Existen algunos paquetes cómo frost y meteor que incluyen funciones para utilizar datos meteorológicos y calcular indices que en algunos casos están asociadosa la actividad agropecuaria. Otro paquete a mencionar es agroclim que según la documentación también incluye el cálculo de una serie de índices agrometeoroógicos fue archivado por CRAN en abril de este año pero el código esta disponible en un repositorio público.
agromet se diferencia de estos paquetes principalmente por 2 características. Por un lado su documentación está escrita integramente en español lo que facilita su uso en Sudamérica. Por el otro lado, sus funciones estan pensadas para ser compatibles con el ecosistema de paquetes que utilizan la filosofía "tidy", lo que facilita posterior manipulación, visualización y uso de los resultados obtenidos con agromet.
(Si aplica) ¿Tu paquete cumple con nuestras guías de Ética, Privacidad de Datos e Investigación de Sujetos Humanos (documento en Inglés)?
No aplica.
Si ya has hecho una consulta de pre-envío, por favor pega el enlace al issue correspondiente, una publicación del foro, u otra discusión. Alternativamente, etiqueta al editor (con @tag) con el que te contactaste.
No aplica.
(Si aplica) Explique las razones de los elementos
pkgcheckque su paquete no puede pasar.Revisiones Técnicas
Tilda los siguientes items para confirmar que los has completado:
Este paquete:
Opciones de Publicación
¿Tienes intenciones de subir este paquete a CRAN?
¿Tienes intenciones de enviar este paquete a Bioconductor?
¿Deseas enviar un Artículo de Aplicaciones sobre tu paquete a Methods in Ecology and Evolution (documento en Inglés)? Si es así:
Opciones para MEE
Código de Conducta
The text was updated successfully, but these errors were encountered: