search

healthinnovation / innovar

Package of functions of the Laboratory of Innovation in Health (InnovaLab) of the Institute of Tropical Medicine "Alexander von Humboldt", Universidad Peruana Cayetano Heredia.
https://healthinnovation.github.io/innovar/
Other
6 stars 6 forks source link

Fix/refactor scale #28

Closed dievillano closed 3 years ago

dievillano commented 3 years ago

Refactoring of scale_lis functions (lis_pal, scale_color_lis and scale_fill_lis). Examples added.

ambarja commented 3 years ago

Hola @DiegoVA92 acabo de revisar los cambios que realizaste a la función y sin duda está bien organizada y entendible, por otro lado quizás más adelante se debería crear un nuevo archivo de R llamado utils donde debería ir puesto toda la paleta de colores para luego solo llamar a las respectivas funciones, y esto se podría complementar con methods de todas las funciones de get_*.

ambarja commented 3 years ago

@brianmsm estado revisando el proyecto de @DiegoVA92 y al parecer existe dos carpetas, estas son:

Estas carpetas son las que generan el error, me pueden explicar si son necesarias, ya que el script si está correcto.

brianmsm commented 3 years ago

@brianmsm estado revisando el proyecto de @DiegoVA92 y al parecer existe dos carpetas, estas son:

* renv

* renv.lock

Estas carpetas son las que generan el error, me pueden explicar si son necesarias, ya que el script si está correcto.

Hasta que podamos determinar mejor cual es el correcto workflow del uso de renv en desarrollo de paquetes, lo mejor sería no utilizarlo. Github actions ya hará toda la revisión de dependencias y uso de versiones cross-plattaform por nosotros.

dievillano commented 3 years ago

@botbarja @brianmsm al parecer para utilizar renv se necesita hacer algunas configuraciones en el github actions, segun como veo aqui: https://rstudio.github.io/renv/articles/ci.html @botbarja podrias chequear segun esa fuente si es muy complicado agregar renv? Sino lo trabajo sin renv, como propone @brianmsm .

ambarja commented 3 years ago

@botbarja @brianmsm al parecer para utilizar renv se necesita hacer algunas configuraciones en el github actions, segun como veo aqui: https://rstudio.github.io/renv/articles/ci.html @botbarja podrias chequear segun esa fuente si es muy complicado agregar renv? Sino lo trabajo sin renv, como propone @brianmsm .

Crees que por esta ocasión la puedes trabajar sin el renv quizás más adelante si sea prioridad para implementar a todo el workflow del paquete.

ambarja commented 3 years ago

@DiegoVA92 puedes trabarjo sin el .renv para poder unirlo ?

dievillano commented 3 years ago

El refactoring se ve vien para mi. Pero veo que hay 2 cosas importantes a tomar en cuenta y/o modificar:

  1. En los files changed veo que se ha introducido configuración de renv. Esto no está en un commit específico. Tener en cuenta que va a afectar esto a toda la rama master y tenemos que estar familiarizados con su manejo.
  2. En el refactoring se eliminaron 2 funciones que estaban exportadas (lis_colors, lis_cols). Sin embargo, ambas funciones siguen exportadas en el NAMESPACE. Es necesario eliminarlo: https://github.com/healthinnovation/lis/blob/667455acecb1ce5e9db4f547ccf45a0a44a73945/NAMESPACE#L14

@ambarja si puedes revisar que más está ocasionando los errores en los checking.

Hola @brianmsm @ambarja resolví estos cambios en los últimos commits. Por fa, cuando puedan lo revisan para ver que todo esté bien. Gracias!