search

ropensci / software-review

rOpenSci Software Peer Review.
286 stars 104 forks source link

Aprendiendo programación en R con la robot Karel #620

Closed mpru closed 1 week ago

mpru commented 6 months ago

Date accepted: 2024-06-19 Nombre de la Persona Encargada: Marcos Prunello Usuario GitHub de la Persona Encargada: !--author1-->@mpru<!--end-author1-- Repositorio: https://github.com/mpru/karel Versión Enviada: 0.1.1.9000 Tipo de Envio: Estándar Editora: !--editor-->@maurolepore<!--end-editor-- Revisores: @vjimenez9, @joelnitta

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

Package: karel
Title: Learning programming with Karel the robot
Version: 0.1.1.9000
Authors@R: 
    person(given = "Marcos",
           family = "Prunello",
           role = c("aut", "cre", "cph"),
           email = "marcosprunello@gmail.com",
           comment = c(ORCID = "0000-0002-9611-527X"))
Description: This is the R implementation of Karel the robot, a programming 
  language created by Dr. R. E. Pattis at Stanford University in 1981. Karel is 
  an useful tool to teach introductory concepts about general programming, such 
  as algorithmic decomposition, conditional statements, loops, etc., in an 
  interactive and fun way, by writing programs to make Karel the robot achieve 
  certain tasks in the world she lives in. Originally based on Pascal, Karel 
  was implemented in many languages through these decades, including 'Java', 'C++', 
  'Ruby' and 'Python'. This is the first package implementing Karel in R.
Depends: R (>= 3.6.0)
Imports:
    purrr,
    dplyr,
    tidyr,
    ggplot2,
    magrittr,
    gganimate,
    cli
License: GPL-2
Encoding: UTF-8
LazyData: true
Language:
  en,
  es
RoxygenNote: 7.2.3
Suggests: 
    testthat (>= 3.0.0),
    knitr,
    rmarkdown
VignetteBuilder: knitr
URL: https://mpru.github.io/karel/
BugReports: https://github.com/mpru/karel/issues/
Config/testthat/edition: 3

Alcance

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 6 months 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 6 months ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 6 months ago

Checks for karel (v0.1.1.9000)

git hash: 4b057e67

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

Package License: GPL-2

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 | 104| |internal |karel | 44| |internal |utils | 9| |internal |methods | 5| |internal |stats | 3| |imports |cli | 21| |imports |dplyr | 10| |imports |ggplot2 | 6| |imports |magrittr | 6| |imports |tidyr | 4| |imports |purrr | 2| |imports |gganimate | 1| |suggests |testthat | NA| |suggests |knitr | NA| |suggests |rmarkdown | 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 (33), message (22), call (17), paste (10), for (4), all (3), list (3), nrow (2), apply (1), array (1), dim (1), emptyenv (1), if (1), integer (1), new.env (1), numeric (1), seq_len (1), which (1)

karel

beepers_present (3), get_beepers_df_row (2), get_pkg_env (2), load_super_karel (2), move (2), right_is_clear (2), turn_around (2), avanzar (1), cargar_super_karel (1), check_user_world (1), check_walls (1), conseguir_amb (1), create_beepers (1), darse_vuelta (1), derecha_abierto (1), draw_karel_df (1), facing_east (1), facing_north (1), facing_south (1), facing_west (1), front_is_blocked (1), front_is_clear (1), generate_world (1), karel_has_beepers (1), karel_has_no_beepers (1), left_is_blocked (1), left_is_clear (1), no_beepers_present (1), pick_beeper (1), plot_static_world (1), put_beeper (1), put_hor_walls (1), right_is_blocked (1), run_actions (1), turn_left (1), turn_right (1)

cli

cli_abort (21)

dplyr

n (4), tibble (3), filter (2), case_when (1)

utils

de (7), data (2)

ggplot2

geom_point (2), scale_x_continuous (2), scale_y_continuous (2)

magrittr

%>% (6)

methods

el (5)

tidyr

expand_grid (4)

stats

time (3)

purrr

pmap_dfr (2)

gganimate

gifski_renderer (1)

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 R (100% in 9 files) and - 1 authors - 12 vignettes - no internal data file - 7 imported packages - 63 exported functions (median 1 lines of code) - 111 non-exported functions in R (median 3 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 | 9| 55.2| | |files_vignettes | 12| 99.6| | |files_tests | 5| 81.7| | |loc_R | 912| 65.7| | |loc_vignettes | 1844| 96.8|TRUE | |loc_tests | 310| 64.9| | |num_vignettes | 12| 99.9|TRUE | |n_fns_r | 174| 87.6| | |n_fns_r_exported | 63| 91.0| | |n_fns_r_not_exported | 111| 85.7| | |n_fns_per_file_r | 10| 86.7| | |num_params_per_fn | 2| 11.9| | |loc_per_fn_r | 1| 0.0|TRUE | |loc_per_fn_r_exp | 1| 0.0|TRUE | |loc_per_fn_r_not_exp | 3| 1.5|TRUE | |rel_whitespace_R | 21| 69.7| | |rel_whitespace_vignettes | 38| 98.6|TRUE | |rel_whitespace_tests | 17| 57.6| | |doclines_per_fn_exp | 66| 77.1| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 66| 71.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 [![pkgcheck](https://github.com/mpru/karel/workflows/pkgcheck/badge.svg)](https://github.com/mpru/karel/actions) [![R-CMD-check](https://github.com/mpru/karel/workflows/R-CMD-check/badge.svg)](https://github.com/mpru/karel/actions) **GitHub Workflow Results** | id|name |conclusion |sha | run_number|date | |----------:|:--------------------------|:----------|:------|----------:|:----------| | 7347999524|pages build and deployment |success |1367b6 | 20|2023-12-28 | | 7347980615|pkgcheck |success |4b057e | 13|2023-12-28 | | 7347980611|pkgdown |success |4b057e | 23|2023-12-28 | | 7347980603|R-CMD-check |success |4b057e | 5|2023-12-28 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) rcmdcheck found no errors, warnings, or notes #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 87.04 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) The following functions have cyclocomplexity >= 15: function | cyclocomplexity --- | --- check_user_world | 28 check_walls | 16 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 53 potential issues: message | number of times --- | --- Avoid library() and require() calls in packages | 4 Lines should not be more than 80 characters. | 45 unexpected symbol | 4

4. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following function name is duplicated in other packages: - - `move` from BacArena, chess, cubing, red, rugarch, seqinr, SpaDES.tools

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

jhollist commented 6 months ago

@mpru Thank you for the submission. I am working on finding a handling editor.

maurolepore commented 5 months ago

@ropensci-review-bot assign @maurolepore as editor

ropensci-review-bot commented 5 months ago

Assigned! @maurolepore is now the editor

maurolepore commented 5 months ago

@mpru, es un placer editar tu paquete.

Pronto empezaré a trabajar en la lista de verificación.

Mientras tanto:

maurolepore commented 5 months ago

@mpru, perdón por la confusión.

Dado que el paquete es multilingüe, el equipo editorial considera que la mejor opción es nominar un/a revisor/a en español y otro/a en inglés.

mpru commented 5 months ago

Hola, @maurolepore, gracias por involucrarte en este envío. Ambos idiomas están bien para mí y me parece genial la idea del equipo editorial. No estoy familiarizado con los nombres de la lista de revisores como para nominar candidatos. ¿Tal vez vos puedas ayudarme con eso?

maurolepore commented 5 months ago

@mpru,

Seguro ayudo con eso. Nuestra guia ofrece varias ideas sobre donde buscar revisore/as:

https://devdevguide.netlify.app/es/softwarereview_editor.es#where-to-look-for-reviewers

Yo usare esa misma guia pero en rOpenSci nos interesa tu opinion.

Cuando encuentres algun/a candidato/a por favor consiera evitar conflictos de interes.

maurolepore commented 5 months ago

Dear @mpru,thanks for your work. Editor checks were super smooth.

You'll see some comments that require your attention. They are labeled ml01, ml02, and so on. The one(s) starting with a checkbox are required. The one(s) starting with a bullet are just for your consideration.

Editor checks:

Editor comments

FIT

rOpenSci accepts this package as part of the Champions program.

VIGNETTES

karel is a new R package ...

TESTS

A search for "test_that(" shows that the test titles seem too general -- https://github.com/search?q=repo%3Ampru%2Fkarel%20test_that(&type=code

And I see many expectations per tests, e.g.:

https://github.com/mpru/karel/blob/master/tests/testthat/test-actions.R

You want to arrange things such that, when a test fails, you’ll know what’s wrong and where in your code to look for the problem. This motivates all our recommendations regarding file organisation, file naming, and the test description. Finally, try to avoid putting too many expectations in one test - it’s better to have more smaller tests than fewer larger tests.

-- https://r-pkgs.org/testing-basics.html#test-organisation

Usually you care about two things when testing an error:

  • Does the code fail? Specifically, does it fail for the right reason?
  • Does the accompanying message make sense to the human who needs to deal with the error?

-- https://r-pkgs.org/testing-basics.html#expectations

For example see https://github.com/search?q=path%3Atests%2Ftestthat+repo%3Ampru%2Fkarel+library%28karel%29&type=code

mpru commented 5 months ago

Thanks Mauro! I already started working in your comments, I'm reviewing and changing my unit tests. I'll suggest reviewers as well.

maurolepore commented 5 months ago

@ropensci-review-bot seeking reviewers

ropensci-review-bot commented 5 months ago

Please add this badge to the README of your package repository:

[![Status at rOpenSci Software Peer Review](https://badges.ropensci.org/620_status.svg)](https://github.com/ropensci/software-review/issues/620)

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

mpru commented 4 months ago

Hi @maurolepore. I'm sorry I couldn't do it sooner but I wanted to let you know that I worked on your comments and think it might be ready to give it another try.

Vignettes

Tests

Others

Suggested reviewers

That's all for now, thanks.

maurolepore commented 4 months ago

@mpru thanks a lot for this! It will make the reviewers's job easier.

Upgraded the version number from 0.1.1.9000 to 0.1.1.9001 (not sure if this is ok).

I think it's OK. That's the kind of increment I see with fledge::bum_version() in my own packages.

I didn't understand how to use Airtable to search for potential reviewers, could it be that the general public doesn't have access to the database and only editors?

I think you're right, and Airtable is only useful to editors with the right access permission.

I'll resume the search for reviewers.

maurolepore commented 4 months ago

@ropensci-review-bot assign @vjimenez9 as reviewer

ropensci-review-bot commented 4 months ago

@vjimenez9 added to the reviewers list. Review due date is 2024-03-14. Thanks @vjimenez9 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 4 months ago

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

maurolepore commented 4 months ago

@vjimenez9, dado que el paquete es multilingüe, el equipo editorial considera que la mejor opción es nominar un/a revisor/a en español y otro/a en inglés. Podias revisarlo en español?

maurolepore commented 3 months ago

@ropensci-review-bot assign @joelnitta as reviewer

ropensci-review-bot commented 3 months ago

@joelnitta added to the reviewers list. Review due date is 2024-03-24. Thanks @joelnitta 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 3 months ago

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

ropensci-review-bot commented 3 months ago

:calendar: @vjimenez9 you have 2 days left before the due date for your review (2024-03-14).

vjimenez9 commented 3 months ago

Hi!

Hoy es el ultimo dia de revisión que tengo. Ya prácticamente revisé todo el paquete y tengo mis comentarios y anotación...Entiendo que los agrego al documento PlatillaDeRevisionROpenSci.md. y subo este documento acá. Cierto? Espero terminar esto en el transcurso de la tarde, pero espero que no haya mucho problema si concluyo mañana por la mañana, porque mi Mac me esta dando problemas con la subida de archivos a github

vjimenez9 commented 3 months ago

Aqui el resultado de mi revisión:

PlatillaDeRevisionROpenSci.md

Revisión de un paquete

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:

Estimación de horas dedicadas a la revisión: 6 horas.

Comentarios de la revisión

Mis notas:

Empece haciendo la instalación en una computadora con SO Ubuntu y me marcó

Warning in install.packages :
  package ‘Karel’ is not available for this version of R

Averigundo un poco, encontré que No hay una versión para ubuntu. Sería util que lo dijeran desde el README.

Luego hice la instalación en una macOS. Les dejo aqui los detalles de los inconvenientes que tuve para realizar la istalación (Solo con fines informativos):

> install.packages("karel")
also installing the dependencies ‘scales’, ‘lpSolve’, ‘ggplot2’, ‘transformr’, ‘tweenr’, ‘gganimate’, ‘gifski’

Warning in install.packages :
  lzma decoder corrupt dataY

Solución: descargar los compilables...

ejecutar_acciones()
ℹ The package "gifski" is required to use the `gifski_renderer`
✖ Would you like to install it?

Me indica que no esta instalado un compilador...y me pide instalar rust...¿Porque ? Bueno pues instalo rust: 

curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh

Al volver intentar me dice:

Error in loadNamespace(x) : there is no package called ‘gifski’

Vuelvo a instalar el paquete, y me dice:

There is a binary version available but
  the source version is later:
        binary   source needs_compilation
gifski 1.6.6-1 1.12.0-2              TRUE

Do you want to install from sources the package which needs compilation? (Yes/no/cancel)

Durante 3 ocasiones le dije si y me volvía a marcar que la instalación se daba con errores, la ultima vez, le dije que no compilara...Y jalo!! jejeje.

Del texto :

En general es muy bonito "el karel" .

Tengo algunas sugerencias de modificación en el texto, que son eso "sugerencias" o comentarios de mi punto de vista. Siempre en el afan de mejorar o hacer mas claro el texto.

====Del texto:

Dice: Desafortunadamente, las computadoras no entienden español ni otro idioma humano. Hay que pasarles las instrucciones en un lenguaje que sean capaces de entender. Para eso debemos aprender algún lenguaje de programación, Comentario: Pero...los lenguajes de programación están escritos en su mayoría en idioma ingles, hay algunos incluso en español...que son idiomas humanos, creo que el problema no es el idioma sino la sintaxis del idioma.

Dice: Ahora bien, ¿por qué debemos estudiar programación en si nos dedicamos a la Estadística? La actividad de los profesionales estadísticos está atravesada en su totalidad por la necesidad de manejar con soltura herramientas informáticas que nos asisten en las distintas etapas de nuestra labor, desde la recolección y depuración de conjuntos de datos, pasando por la aplicación de distintas metodologías de análisis, hasta la comunicación efectiva de los resultados. Comentario: Me parece que no todo el mundo al que va dirigido este paquete es del area de estadística...O aunque aplique estadística se identifica como analista de datos o científicos de datos...¿No sería mas conveniente remplazar la pregunta por: ¿Porqué debemos aprender a programar cuando queremos analizar datos, generar estadísticas, gráficas y conclusiones? La actividad de los profesionales estadísticos, de ciencia de Datos, o cualquier persona que requiera hacer análisis de datos se potencia con el manejo de herramientas bioinformáticas y capacidades de programación que nos asisten...

Dice: en este tutorial estudiaremos los conceptos básicos de esta disciplina

Sugerencia: en este tutorial estudiaremos los conceptos básicos de los lenguajes de programación.

Dice: El lengua de programación en el que se basa ... Debe decir: El lenguaje de programación en el que se basa ...

Dice: que viene bien tener presentea.

Debe decir: que viene bien tener presente.

Dice: En programación, el lenguaje artificial e informal

Comentario:

No estoy tan de acuerdo que el pseudocodigo sea un "lenguaje artificial e informal".

Dice: El programa se guarda en un archivo con un nombre generalmente dividido en dos partes por un punto, por ejemplo: mi_primer_programa.R. La primera parte es la raíz del nombre con la cual podemos describir el contenido del archivo. La segunda parte es indicativa del uso del archivo, por ejemplo, .R indica que contiene un programa escrito en el lenguaje R. El proceso general de ingresar o modificar el contenido de un archivo se denomina edición.

Sugerencia: La codificación del programa se guarda en un archivo de texto plano con un identificador generalmente dividido en dos partes por un punto, por ejemplo: mi_primer_programa.R. La primera parte es el nombre del archivo. La segunda parte es indicativa del lenguaje que puede interpretar las instrucciones, por ejemplo, .R indica que contiene un programa escrito en el lenguaje R. El proceso general de escribir o modificar las instrucciones en un archivo se denomina edición.

En la sección de los errores creo que sería util hablar de que hay dos tipos de errores: los lógicos y los sintácticos. Estos últimos tienen que ver cuándo las instrucciones, o nombres de variables no son correctamente escritos y el programa no puede "interpretarlos". Los errores lógicos, se generan con instrucciones que el programa puede interpretar, pero que realizan cosas que no queremos.

Estoy de acuerdo que el procesador es el lenguaje que "procesa, interpreta y ejecuta" las instrucciones....Pero la frase "Todos los elementos disponibles para ser utilizados por el procesador constituyen su entorno o ambiente." Yo digo que no son elementos disponibles para el procesador, son elementos disponibles para el programador.

En la sección, organización de RStudio:

Me parece que hay que tener cuidado con decir que el editor es "un Word muy simple", puede sugerir erróneamente que se puede usar word para editar un archivo, cosa que es completamente errónea. Necesitamos un poderoso editor de texto plano como lo proporciona RStudio que puede hacer muchísimas cosas que un "simple word" no puede hacer durante la creación de un archivo de programación.

Dice: Abajo está la consola. Es la ventana que se comunica con R

Comentario: La consola es el ambiente interactivo entre R y el usuario.

Dice: Environment (ambiente): muestra todos los elementos que componen al ambiente o entorno.

Sugerencia: Environment (ambiente): muestra todos objetos generados en la sesión presente

Dice: History (historial): lista todas las instrucciones que R ha corrido anteriormente.

Sugerencia: History (historial): lista todas las instrucciones que el usuario ha ejecutado.

(Porque en los sistemas mutiusuario no vez todo lo que ha ejecuta R, solo lo que tu usuario ha ejecutado)

Dice: Packages: listado de los “paquetes” que tenemos instalados (ver más adelante). Debe decir: Packages: Herramienta de instalación, actualización y carga de los paquetes de R.

Tu ejemplo de la consola me confunde un poco:

1 + 2
#> [1] 3
5 * 3
#> [1] 15
exp(2)

¿No debería la instrucción ir después del prompt ">". Y justo la respuesta de R ir sin prompt? O ¿que es #> ? En todo caso, describe al usuario cual es el prompt del sistema que esta en espera de una instrucción y con cual prompt (si existe) , R te muestra los resultados

En la sección conociendo a karel

La función generar_mundo tiene un argumento "mundo001". Podrias aprovechar este hecho para introducir el concepto de "argumento", y porque los paréntesis vacíos a veces es la ausencia de argumentos o la existencia de argumentos definidos por default.

Descomposición algorítmica

Este es un tema muy importante y muy complejo, pero en el flujo de tu documentación siento un salto cuántico importante. Pasan de 0 a 100 en un solo acelerón...Pero no tengo una propuesta concreta de como "deshacelerar"

En esta sección también varias aseveraciones que me parecen desafortunadas. Por poner solo un ejemplo (hay varios) : Dice Es importante notar que, para que podamos usar la función girar_izquierda(), la misma tiene que ser definida y evaluada por R antes de que hagamos uso de la misma para resolver nuestro problema:

Comentario:

Yo creo en este caso, la función no es definida por R, es definida por el usuario, y esta definición debe estar "antes" de ser invocada en un programa de R.

Documentación de los subalgoritmos

En el propio texto indicas la importancia de documentar, para que se usa tu función y en el ejemplo nunca se especifica para qué sirve.

Nunca se indico qué significan los # al inicio de la linea y sería muy util para los usuarios, así que se puede aprovechar esta sección agregando un comentario que explica que la documentación se introduce como "comentarios" que son antecedidos por #

Podrías poner al menos una liga con lineamientos para la documentación de funciones (por ejemplo: https://combine-australia.github.io/r-pkg-dev/documenting-functions.html). O cualquier otra

Estructuras condicionales simples

Dice: se procede a ejecutar las acciones encerradas por esta estructura.

Sugiero que diga: se procede a ejecutar las acciones delimitadas entre las {} que definen el cuerpo de esta estructura.

Dice: Si la condición no se verifica,

Debe decir: Si la condición es falsa

Comentario: siempre se verifica, y esta verificación a veces arroja verdadero a veces falso. Pero veo que a lo largo del texto se usa muchas veces "verificar" como sinónimo de la evaluación verdadera, entonces, quizá solo al inicio, indicar esta "interpretación" de la palabra verificar. Algo así como: "Decimos que un enunciado se verifica, cuando su evaluación lógica es verdadera"

Dice: Mantener la prolijidad en nuestros programas es esencial. Sugiero que diga: Mantener la claridad en nuestros programas es esencial.

Dice: La letra i recibe el nombre de variable de iteración

Comentario: No hemos definido que es una variable. Mi sugerencia es incluir desde la primera o segunda seccion

Dice: El bloque de instrucciones se repite tantas veces como i tarde en llegar a 5 partiendo desde 1.

Sugerencia: El bloque de instrucciones se repite tantas veces como i tarde en llegar a 5 partiendo desde 1, esto se define con un rango de valores en R usando la nomenclatura 1:5.

Dice:

for (<variable> in <valor1>:<valor2>) {
    ...Acción/es...
}

Sugerencia:

for (<variable> in <valorInicial>:<valorFinal>) {
    ...Acción/es...
}
maurolepore commented 3 months ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/620#issuecomment-2000346572 time 6

ropensci-review-bot commented 3 months ago

Logged review for vjimenez9 (hours: 6)

maurolepore commented 3 months ago

Muchas gracias @vjimenez9

Edite tu comentario para mostrar tu revision directamente aca. Es la forma mas habitual. Perdon que me olvide de aclararlo.

ropensci-review-bot commented 3 months ago

:calendar: @joelnitta you have 2 days left before the due date for your review (2024-03-24).

mpru commented 3 months ago

¡Hola @vjimenez9!

Muchísimas gracias por haber aceptado revisar este paquete y por el esfuerzo y dedicación puestos en esta tarea. Tus observaciones son muy útiles. A continuación voy a ir respondiéndolas:

Documentación

Instrucciones de instalación Nota: En README no se especifica que no es posible instalarlo en Ubuntu, tampoco que se requiere tener instalado RUST

Acerca de la instalación en Ubuntu, no he podido reproducir este inconveniente. De hecho, el paquete ha sido desarrollado en Ubuntu, ya que es el sistema operativo con el que trabajo. Utilicé rhub::check_on_ubuntu() para evaluarlo en un medio externo y al parecer la evaluación fue exitosa.

De todas formas, veo en la salida que muestras que dice "Karel" con mayúscula, el paquete se llama "karel" con minúscula. ¿Puede que sea esa la razón por la cual no lo pudiste instalar?

Tampoco hemos detectado problemas de instalación en Windows o en Posit Cloud, que son los entornos más utilizamos en clase. Además de correr pkgcheck, he utilizado devtools para chequear el paquete con check_win, abajo pongo los enlaces, no parece haber habido algún problema relacionado a la instalación:

Con respecto a macOS, he corrido devtools::check_mac_release() y no parece haber detectado problemas con la instalación. Lamentablemente ni yo ni las personas en mi entorno usan macOS como para poder hacer pruebas locales, no sé cómo afrontar esta parte, agradezco sugerencias.

Funcionalidad

El punto Directrices de empaque no ha sido tildado, ¿podrías orientarme acerca de qué aspecto revisar? Entiendo que relacionado con esto debe estar el comentario de la revisón que dice:

¿El paquete cumple con la guía de embalaje rOpenSci? en general si

Comentarios de la revisión

¿El código cumple con los principios generales de la guía de revisión de Mozilla? No me queda claro si hubo alguna revisión por pares, o como dice la guia es un Flying Solo. Por otro lado me parece que la meta es clara y se cumple.

El paquete no ha tenido otra revisión por fuera de rOpenSci, pero participar del presente proceso de revisión de rOpenSci a través del programa Champions satisface este requisito.

¿Hay duplicación de código en el paquete que debería reducirse? No necesariamente se duplica, pero hay lugares donde se puede obtimizar. Por ejemplo donde se dibuja a Karel, se pueden definir un grupo de vectores que se asignen al tible en función de la dirección, en lugar de crear los 4 tibles por separado. Tambien cuando se hacen los chequeos ...se podria genear una función a la que se llama multiples veces con el objeto a checar... etc.

Cambiar la estructura de objetos diseñada para graficar a Karel en cada paso implicaría reformar todo el código base del paquete. En su uso no se me han presentado situaciones que me hicieran pensar que una reestructuración sea necesaria. ¿Consideras estrictamente necesario este cambio? ¿O es sólo una observación?

Sección Mis notas

Averigundo un poco, encontré que No hay una versión para ubuntu. Sería util que lo dijeran desde el README. Luego hice la instalación en una macOS. Les dejo aqui los detalles de los inconvenientes que tuve para realizar la istalación (Solo con fines informativos):

Ver mi comentario anterior al respecto.

Sugerencias de modificación en el texto.

Con respecto a tus sugerencias de mejoras para el texto de los tutoriales, te agradezco por cada una de ellas y por haberlo leído con atención. Incorporé casi todas en el commit https://github.com/mpru/karel/commit/fb65a4c40118cff94a756968458f459ec31335a9, dejando de lado sólo algunas porque se refieren a cuestiones que en realidad estaban resueltas o presentes en otras partes, o por diferencia de criterio.

Bueno, esto es lo que tengo para responder con respecto a tu revisión. Te agradezco nuevamente por ĺa buena voluntad y disposición de evaluar mi trabajo, espero que podamos seguir trabajando hasta lograr el objetivo.

vjimenez9 commented 3 months ago

Estimado @mpru:

Tienes razón, es muy probable que el error fue por la K mayúscula de Karel en la instalación de ubuntu. Esta fue la razón por la que no puse el tilde en "directrices de empaque". porque no me funcionaba en ubuntu y hubo complicaciones en mac, básicamente fue esa la razón.

Reestructurar el código para reducir duplicidad o simplificarlo, es una sugerencia, y no un requisito obligatorio. No tengo problema con que se quede como esta. Finalmente funciona, y funciona bien.

Tu herramienta es muy bonita, pero finalmente tengo ya varios años generando material para la enseñanza de programación y por eso me atreví a SUGERIR tan detalladamente en el texto, siempre con el objetivo de mejorarlo. Me parece que la mayoría de lo vital quedo plasmado. Supongo que con esto terminan mis sugerencias y no me queda mas que felicitarte.

maurolepore commented 2 months ago

@joelnitta just a friendly reminder that we're looking forward to your review :-)

joelnitta commented 2 months ago

Package Review

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 5

Review Comments

To-do list

UI

Many of the error messages are preceded by cli::cli_rule(). While of course informative error messages are important, care should be taken that the package is not overly chatty. I don't see why the cli::cli_rule() is needed.

Tests

For expect_error(), it is a good idea to indicate the expected error message using the regexp argument so that you know the error is actually the expected one and not another error. This also makes the intention of the test clearer, for example the first test of test-set_up.R.

There is a lot of repeated code in test-set_up.R where each list of world elements (world_test) is being defined slightly differently. I think this could be simplified by starting with a single world_test_template list, then modifying it for each test to check that the correct error message is generated. For example:

world_test_template <- list(
  nx = 6, ny = 4,
  hor_walls = dplyr::tibble(x = 3, y = 1, lgth = 3),
  ver_walls = dplyr::tibble(x = 3, y = 0, lgth = 1),
  karel_x = 1, karel_y = 1,
  karel_dir = 1,
  beepers_x = 2, beepers_y = 1, beepers_n = 1,
  beepers_bag = Inf)

test_that("The generic world template works correctly before modifying it
           in subsequent tests", {
  expect_no_error(generar_mundo(world_test_template))
  expect_no_error(generate_world(world_test_template))
})

test_that("Karel's initial direction is required when creating a new world
           provided as a list by the user.", {
  world_test <- world_test_template
  world_test$karel_x <- NULL
  expect_error(generate_world(world_test), "karel_x is missing")
})

test_that("The correct number of avenues (nx) is required when creating a newa
           world provided as a list by the user.", {
  world_test <- world_test_template
  world_test$nx <- 6.4
  expect_error(
    generate_world(world_test),
    "nx, the number of avenues, must be numeric of length 1")
})

It would be good to add a test coverage badge so we can verify what percentage of functions are included in tests.

Examples

I recommend use of if (interactive()) instead of if(FALSE) for skipping example code like run_actions() that should not be run during checks. if (interactive()) is more explicit in its intent than if(FALSE), and will work when users copy-paste the example code. Sometimes CRAN checks can be picky about how you specify which code to skip in examples.

Multilingual documentation

As far as I know, this package is unique in its approach to multilingual functionality and documentation. I applaud the author for this commitment to breaking down linguistic barriers. However, there are several aspects to consider carefully here.

I am a little concerned about the multilingual aliases for function names. First, I think ultimately it could be counter-productive for the learners. For better or worse, English is the standard language used in programming. If the goal is to teach students programming, at some point they will have to deal with words that are in English. We can even see this in karel lessons: although the karel functions have names in Spanish, the learners still need to use base R code with syntax like if(), else(), function(), etc. It may be better to get them used to writing R syntax in English from the start instead of mixing English and Spanish (or another language). Furthermore, CRAN does not allow non-ASCII characters in R code, so it will be difficult or impossible to display function names in some non-English languages. Finally, using only one name per function makes the package easier to maintain. Currently, entire test files are duplicated and must be maintained due to having function aliases. However, as I mention below, one argument in favor of function aliases is that it allows you to maintain help files in a different languages... this is a complicated topic!

Apart from function names, I think localization of function UI and documentation is absolutely a plus and should be done. I see this as falling into three main categories: localization of UI, documentation of functions in the package (help files), and the package website. This package is pushing the limits of what R can do in terms of multilingual functionality, and as I describe below, mature solutions for each of these are not yet available in some cases.

For localizing the UI (function messages), I recommend the potools package (and PO files). This provides a very clean way to localize a package. Once it is set up, the user should have a seamless linguistic experience: if they are using a computer with a Spanish locale, all of the package messages will be in Spanish; same for English, etc. To get started with potools, I recommend this blog post by Maëlle Salmon. This is the localization aspect that has the most mature solution available.

@eliocamp is working on a package to localize help file contents, rhelpi18n. It should eventually provide a similar seamless experience as UI localization with PO tools, but unfortunately does not seem ready for production use yet. So honestly I am not sure what to do here. I wish there was a way to provide a help file in an alternate language that did not require a different function name. Actually, this may be a point in favor of keeping function aliases: it actually allows you to provide help files in different languages.

For the website, it would be ideal if there was a "language button" that could be clicked to switch between languages on a given page. With the current setup, both languages (Spanish and English) are displayed in a single webpage, but a given user probably only needs to see one or the other. Also, while this approach works OK for two languages, it would likely become unusable if any more were added. It would be great if pkgdown fully supported multilingual websites, but unfortunately this does not seem to be the case. Although pkgdown can provide website elements in different languages by setting a YAML parameter, it is limited to making a single webpage in one language. Since this is currently set to Spanish, various website elements appear in Spanish even though some of the content is in English. So, in absence of a genuine solution from pkgdown, I wonder if a work-around via forking the package and maintaining a website in a different language from there could be an option. This is not ideal, but it would scale better if multiple languages were to be implemented.

Vignettes

This package is also unique in that much of the vignette contents are actually lessons teaching programming in R with karel, rather just than how to use the karel functions. I think it may be preferable to split out the lessons teaching R into their own website. This would make maintaining the package easier. I am not sure of the optimal format, but one that I can recommend is the Carpentries lesson template (known as "the Workbench"). I could easily see the vignette contents being converted into a Carpentries lesson and hosted on the Carpentries Incubator. Please consider this.

I also noticed that much of the lesson content is focused on control flow. While this makes sense as a general feature of programming languages, R these days is often used for data analysis, which may not necessarily put as much emphasis on control flow. I personally almost never use while(), and try to use apply or purrr::map_() family functions instead of for loops. I wonder if there may be someway to use karel to help teach data analysis, as it is in relatively high demand.

Other random comments

maurolepore commented 2 months ago

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

ropensci-review-bot commented 2 months ago

Logged review for joelnitta (hours: 5)

maurolepore commented 2 months ago

Thanks @vjimenez9 and @joelnitta for your reviews!

@mpru I see you already responded to @vjimenez9's reivew (in https://github.com/ropensci/software-review/issues/620#issuecomment-2027821275). If that's your final response to that review, then please simply acknowledge it; else please add whatever you feel necessary and/or refer to https://github.com/ropensci/software-review/issues/620#issuecomment-2027821275 when appropriate.

Also please respond to @joelnitta's review.

We recommend responding within the next 2 weeks.

In general aim for 3 weeks for review, 2 weeks for subsequent changes, and 1 week for reviewer approval of changes. --https://devdevguide.netlify.app/softwarereview_editor#during-review

ropensci-review-bot commented 2 months ago

@mpru: please post your response with @ropensci-review-bot submit response <url to issue comment> if you haven't done so already (this is an automatic reminder).

Here's the author guide for response. https://devguide.ropensci.org/authors-guide.html

mpru commented 2 months ago

@maurolepore hoy se cumplen las dos semanas de plazo para completar mis respuestas, pero aún no he tenido oportunidad de hacerlo. Pido disculpas por la demora, espero hacerlo a la brevedad.

maurolepore commented 2 months ago

OK, gracias por avisar :-)

mpru commented 1 month ago

@maurolepore, apologies for the delay, here I acknowledge my previous response to Veronica and proceed to respond to Joel.

Response to @vjimenez9

My response to @vjimenez9 was stated in this comment. I really thank you again for your comments and suggestions.

Response to @joelnitta

Thank you very much for all the work and commitment put into reviewing my package. I really appreciate all the comments and corrections made, many of which I would have liked to have read during the initial stages of the package's development, since they refer to structural issues of the package. Below I am going to respond to your comments one by one, indicating the changes applied to the package or the reason why I have not implemented, at least for now, some of the suggestions.

Review Comments

To-do list

Response. Updated README in https://github.com/mpru/karel/commit/d07dfcbaebdb15c85068264e7fda4c534a2c456a to try to address it more explicitly.

UI

Many of the error messages are preceded by cli::cli_rule(). While of course informative error messages are important, care should be taken that the package is not overly chatty. I don't see why the cli::cli_rule() is needed.

Response. _Removed all calls to cli_rule() in https://github.com/mpru/karel/commit/a644c01f24e72202e132580d30c614e11ef417fa_

Tests

For expect_error(), it is a good idea to indicate the expected error message using the regexp argument so that you know the error is actually the expected one and not another error. This also makes the intention of the test clearer, for example the first test of test-set_up.R.

There is a lot of repeated code in test-set_up.R where each list of world elements (world_test) is being defined slightly differently. I think this could be simplified by starting with a single world_test_template list, then modifying it for each test to check that the correct error message is generated. For example:

world_test_template <- list(
  nx = 6, ny = 4,
  hor_walls = dplyr::tibble(x = 3, y = 1, lgth = 3),
  ver_walls = dplyr::tibble(x = 3, y = 0, lgth = 1),
  karel_x = 1, karel_y = 1,
  karel_dir = 1,
  beepers_x = 2, beepers_y = 1, beepers_n = 1,
  beepers_bag = Inf)

test_that("The generic world template works correctly before modifying it
           in subsequent tests", {
  expect_no_error(generar_mundo(world_test_template))
  expect_no_error(generate_world(world_test_template))
})

test_that("Karel's initial direction is required when creating a new world
           provided as a list by the user.", {
  world_test <- world_test_template
  world_test$karel_x <- NULL
  expect_error(generate_world(world_test), "karel_x is missing")
})

test_that("The correct number of avenues (nx) is required when creating a newa
           world provided as a list by the user.", {
  world_test <- world_test_template
  world_test$nx <- 6.4
  expect_error(
    generate_world(world_test),
    "nx, the number of avenues, must be numeric of length 1")
})

Response: Changed tests to implement this in https://github.com/mpru/karel/commit/ae770bf786724da4ba9b73393a45e3ca2f9218fe and https://github.com/mpru/karel/commit/a995fa86dce5687c191764316031e592bd58efd2

It would be good to add a test coverage badge so we can verify what percentage of functions are included in tests.

Response: Implemented in https://github.com/mpru/karel/commit/9c1b69c4a6888a53c6db2f1668e861ffd31c6267

Examples

I recommend use of if (interactive()) instead of if(FALSE) for skipping example code like run_actions() that should not be run during checks. if (interactive()) is more explicit in its intent than if(FALSE), and will work when users copy-paste the example code. Sometimes CRAN checks can be picky about how you specify which code to skip in examples.

Response. Fixed in https://github.com/mpru/karel/commit/cb143b56abe312f92968eba8b7d10bdbc4ceae3c

Multilingual documentation

As far as I know, this package is unique in its approach to multilingual functionality and documentation. I applaud the author for this commitment to breaking down linguistic barriers. However, there are several aspects to consider carefully here.

I am a little concerned about the multilingual aliases for function names. First, I think ultimately it could be counter-productive for the learners. For better or worse, English is the standard language used in programming. If the goal is to teach students programming, at some point they will have to deal with words that are in English. We can even see this in karel lessons: although the karel functions have names in Spanish, the learners still need to use base R code with syntax like if(), else(), function(), etc. It may be better to get them used to writing R syntax in English from the start instead of mixing English and Spanish (or another language). Furthermore, CRAN does not allow non-ASCII characters in R code, so it will be difficult or impossible to display function names in some non-English languages. Finally, using only one name per function makes the package easier to maintain. Currently, entire test files are duplicated and must be maintained due to having function aliases. However, as I mention below, one argument in favor of function aliases is that it allows you to maintain help files in a different languages... this is a complicated topic!

Response. I share your point of view. For better or worse, and sooner or later, our students must learn to use English, not only for programming, but because much of the new literature in the discipline is written in English. However, for the context in which we use this package (or in which I think others may use it, I write more about this later), I think there is an important difference between using and learning just a few English terms (if, else, function, etc.) and having everything else in Spanish, and having absolutely everything in English. With aliases, students can think about and discuss what actions Karel should take to solve a problem and write them down in their own language (e.g., girar a la derecha, juntar, poner), while also being able to consult help and see examples in their own language. I understand that this does not solve the need for English in the medium term, but I like having the possibility of using the environment mostly in Spanish as a first approach.

Apart from function names, I think localization of function UI and documentation is absolutely a plus and should be done. I see this as falling into three main categories: localization of UI, documentation of functions in the package (help files), and the package website. This package is pushing the limits of what R can do in terms of multilingual functionality, and as I describe below, mature solutions for each of these are not yet available in some cases.

For localizing the UI (function messages), I recommend the potools package (and PO files). This provides a very clean way to localize a package. Once it is set up, the user should have a seamless linguistic experience: if they are using a computer with a Spanish locale, all of the package messages will be in Spanish; same for English, etc. To get started with potools, I recommend this blog post by Maëlle Salmon. This is the localization aspect that has the most mature solution available.

Response: Thanks for commenting on the potools package, I didn't know about it. I think it's something I could explore, and I'd like to, although this would involve changing the current structure of the package extensively and in the short term I don't think I can do it. However, this approach goes hand in hand with having all function names written in one language (which should be English, following the previous reasoning that the rest of the statements like if, function, for, etc., are in English), but I really want and like the names of the functions to be in Spanish, and to use them in Spanish with my students, even though this incurs the alias system with the limitations that we discussed before.

@eliocamp is working on a package to localize help file contents, rhelpi18n. It should eventually provide a similar seamless experience as UI localization with PO tools, but unfortunately does not seem ready for production use yet. So honestly I am not sure what to do here. I wish there was a way to provide a help file in an alternate language that did not require a different function name. Actually, this may be a point in favor of keeping function aliases: it actually allows you to provide help files in different languages.

Response: What a nice project you mentioned. I seem to understand that the system proposed by the rhelpi18n package requires writing the documentation in different languages, which is also what I do now in my package, but saving these translations in different packages. That has the disadvantage of having to install one more package depending on the language you want to use it in, but it is an advantage because it is easier to maintain, and because eventually including more and more languages under the current karel concept would make the package become heavier than allowed. If this approach matures we could use it to have translations of the help pages, but again we would still have a single English name for each function, which I consider to be a limitation. I think I prefer karel's current approach for the moment.

For the website, it would be ideal if there was a "language button" that could be clicked to switch between languages on a given page. With the current setup, both languages (Spanish and English) are displayed in a single webpage, but a given user probably only needs to see one or the other. Also, while this approach works OK for two languages, it would likely become unusable if any more were added. It would be great if pkgdown fully supported multilingual websites, but unfortunately this does not seem to be the case. Although pkgdown can provide website elements in different languages by setting a YAML parameter, it is limited to making a single webpage in one language. Since this is currently set to Spanish, various website elements appear in Spanish even though some of the content is in English. So, in absence of a genuine solution from pkgdown, I wonder if a work-around via forking the package and maintaining a website in a different language from there could be an option. This is not ideal, but it would scale better if multiple languages were to be implemented.

Response. Yes, every time I see the website I think exactly that, with two languages it is fairly ok, but if someone really wanted to keep adding languages, it would quickly become impractical. Hopefully at some point pkgdown will support support for multilingual pages, that would be great. It seems to me that the forking idea could work if more languages are added, including links between the different webpages, although I'm not sure about the source code living in several kind of "official" repos. I think for now I'm going to keep the site as planned now.

Vignettes

This package is also unique in that much of the vignette contents are actually lessons teaching programming in R with karel, rather just than how to use the karel functions. I think it may be preferable to split out the lessons teaching R into their own website. This would make maintaining the package easier. I am not sure of the optimal format, but one that I can recommend is the Carpentries lesson template (known as "the Workbench"). I could easily see the vignette contents being converted into a Carpentries lesson and hosted on the Carpentries Incubator. Please consider this.

Response: I agree with this observation, I made use of the vignettes in a way that is outside the norm for R packages, turning them into lessons. In fact, I use these vignettes as study material for the students I use this material with. This also turns out to be an extra burden to achieve the objective of having all the documentation multilingual, adding a new language would mean translating all the vignettes. However, for the moment I prioritize making this project self-contained, and having my students rely on a single web page for the package and their lessons. I'd be interested in exploring your suggestions about including this in the Carpentries Incubator in the medium term, but it's not something I can pursue right away.

I also noticed that much of the lesson content is focused on control flow. While this makes sense as a general feature of programming languages, R these days is often used for data analysis, which may not necessarily put as much emphasis on control flow. I personally almost never use while(), and try to use apply or purrr::map_() family functions instead of for loops. I wonder if there may be someway to use karel to help teach data analysis, as it is in relatively high demand.

Response: What you notice has been chosen on purpose. To better answer, I must give some context about my use of this package. At my University, this package has been used in an informal pre-university course for some years now for students who will enter the Bachelor's degree in Statistics and who do not have any prior knowledge of programming. The objective is to teach them general programming concepts (applicable to any language) but in the main language that they will begin to use in the immediate future in their studies, R. That's why tools for data analysis or to improve some aspects of coding (like the apply or map examples you mention) are not included in the vignettes. They study all this and much more in the degree. In this course we only see these general programming ideas, not oriented to data analysis, in the environment that they will later use for data analysis. Perhaps the question that I should asked to myself is whether, thinking that the vignettes/lessons can be used in other contexts, it should not include other notions, but I think not, since in this way it remains limited and faithful to its original purpose.

Other random comments

maurolepore commented 1 month ago

¡Gracias @mpru!

@vjimenez9 y @joelnitta, ¿podrían por favor informarnos si recomiendan más cambios o indicar su aprobación utilizando esta plantilla?

--

Thanks @mpru!

@vjimenez9 and @joelnitta can you please let us know if you recommend further changes or indicate your approval using this template?

joelnitta commented 1 month ago

Reviewer Response

Final approval (post-review)

Estimated hours spent reviewing: 5

maurolepore commented 2 weeks ago

@vjimenez9

Solo para recordarte que esperamos tu respuesta.

¿Podrías por favor informarnos si recomiendas más cambios o indicar tu aprobación utilizando esta plantilla?

maurolepore commented 1 week ago

@vjimenez9

Revisando los comentarios veo este comentario por el que asumo tu aprovacion:

Me parece que la mayoría de lo vital quedo plasmado. Supongo que con esto terminan mis sugerencias y no me queda mas que felicitarte. -- @vjimenez9 en https://github.com/ropensci/software-review/issues/620#issuecomment-2028888781

Para no demorar la publicacion voy a formalizar la aprobacion. Si tenes alguna sugerencia mas, es bienvenida pero sugiero que @mpru la considere luego de la publicacion.

De todos modos, cuando puedas, por favor completa esta plantilla asi registramos formalmente tu esfuerzo.

Muchisimas gracias por tu trabajo.

@vjimenez9

Reviewing the comments, I see this one from which I assume your approval (translated with ChatGTP):

It seems to me that most of the essential points have been addressed. I suppose this concludes my suggestions, and I can only congratulate you. -- @vjimenez9 on https://github.com/ropensci/software-review/issues/620#issuecomment-2028888781

To avoid delaying the publication, I will formalize the approval. If you have any further suggestions, they are welcome but I suggest that @mpru consider them after the publication.

In any case, when you can, please complete this template so we can formally record your efforts.

Thank you very much for your work.

maurolepore commented 1 week ago

@ropensci-review-bot approve karel

ropensci-review-bot commented 1 week ago

Approved! Thanks @mpru for submitting and @vjimenez9, @joelnitta 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.