annagraff / densify

GNU Affero General Public License v3.0
0 stars 0 forks source link

JOSS review 1 #7

Closed elenlefoll closed 1 week ago

elenlefoll commented 1 month ago

Hi!

I am reviewing your JOSS paper on this package (https://github.com/openjournals/joss-reviews/issues/7024) and I am encountering an error on installation:

install_github('annagraff/densify', build_vignettes = T)
Downloading GitHub repo annagraff/densify@HEAD
── R CMD build ─────────────────────────────────
✔  checking for file ‘/private/var/folders/_x/ycbk0by91blbvnqkbtv105fw0000gp/T/Rtmp3fjUdu/remotesfb4b4e143e60/annagraff-densify-3e48e73/DESCRIPTION’ ...
─  preparing ‘densify’:
✔  checking DESCRIPTION meta-information ...
─  installing the package to build vignettes
E  creating vignettes (3s)
   --- re-building ‘densify.Rmd’ using rmarkdown
   Error: processing vignette 'densify.Rmd' failed with diagnostics:
   there is no package called ‘bookdown’
   --- failed re-building ‘densify.Rmd’

   SUMMARY: processing the following file failed:
     ‘densify.Rmd’

   Error: Vignette re-building failed.
   Execution halted
Error: Failed to install 'densify' from GitHub:
  ! System command 'R' failed

I am running:

> session_info() ─ Session info ─────────────────────────────── setting value version R version 4.4.1 (2024-06-14) os macOS Sonoma 14.5 system aarch64, darwin20 ui RStudio language (EN) collate en_US.UTF-8 ctype en_US.UTF-8 tz Europe/Madrid date 2024-07-24 rstudio 2024.04.2+764 Chocolate Cosmos (desktop) pandoc NA

Any ideas what is going wrong?

elenlefoll commented 1 month ago

Also, I wanted to point out that the title of your JOSS paper is potentially misleading: "densify: An R package to prune sparse data frames of typological linguistic data" as, to me, the construction "to prune X of Y" means to get rid of Y. In other words, the title sounds as if {densify} can be used to remove typological linguistic data from sparse data frames.

annagraff commented 1 month ago

Hello! Regarding your error, have you tried simply installing the bookdown package?

install.packages("bookdown") remotes::install_github('annagraff/densify', build_vignettes = TRUE)

Regarding the title - happy to put that on the list of things to update!

elenlefoll commented 1 month ago

Yes, after manually installing bookdown, the installation works.

elenlefoll commented 1 month ago

The first "demo" command in the paper returns a warning message for me. Is it intended?

> example_result <- densify(data = WALS,
+                           taxonomy = glottolog_languoids,
+                           taxon_id = "Glottocode",
+                           density_mean = "log_odds",
+                           min_variability = 3,
+                           limits = list(min_coding_density = 1),
+                           density_mean_weights = list(coding = 1, taxonomy = 1))
Warning message:                                                      
In densify(data = WALS, taxonomy = glottolog_languoids, taxon_id = "Glottocode",  :
  ! in `densify()`: no `cols` argument specified,
  using all columns as variables
ℹ use `cols = <tidy column spec>` to silence
  this warning
elenlefoll commented 1 month ago

I was able to tick nearly all the boxes for my review except two that I was not sure about.

  1. Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  2. Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support.

Regarding 2), I saw that you have guidelines for third party wishing to report issues on the repo landing page, but not for the other two cases.

tzakharko commented 1 month ago

Hi @elenlefoll, thank you for your feedback!