Review for JOSS

[ibarraespinosa]

hello @jonsampedro, how are you? I hope everything is fine.

I`m starting this issue to review your article, according to the guidelines suggestions as mentioned here https://github.com/openjournals/joss-reviews/issues/3820

I will be editing this issue and adding more items as my review progress.

[Checklist about academic merit of the paper]

Authors have been developing applications to estimate the health and agricultural effects of air pollution using integrated models such as GCAM and TM5-FASST. Personally, I`m very excited about the package and I~m wondering if I can use ith with my own analyses for Brazil. The manuscript looks very good, too concise I would say. However:

1. The statement of need is OK. The main flaw of the manuscript is that does not provide any (at least simplified) example showing how to run the package.
2. Furthermore, the authors do not mention other tools that might perform similar calculations.

Please, consider these suggestions in your manuscript.

Checklist The JOSS paper

• [x] A list of the authors of the software and their affiliations.
• [x] A summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience.
• [x] A clear statement of need that illustrates the purpose of the software.
• [x] A description of how this software compares to other commonly-used packages in this research area.
• [x] Mentions (if applicable) of any ongoing research projects using the software or recent scholarly publications enabled by it.
• [x] A list of key references including a link to the software archive.
• [x] Note the paper should not include software documentation such as API (Application Programming Interface) functionality, as this should be outlined in the software documentation.

Checklist license

• [x] Acceptable: A plain-text LICENSE or COPYING file with the contents of an OSI approved license

Checklist Substantial scholarly effort

• [x] Age of software (is this a well-established software project) / length of commit history.
• [x] Number of commits.
• [x] Number of authors.
• [x] Lines of code (LOC): These statistics are usually reported by Whedon in the pre-review issue thread.
• [x] Whether the software has already been cited in academic papers.
• [x] Whether the software is sufficiently useful that it is likely to be cited by other researchers working in this domain.

Statement of need

• [x] The authors should clearly state what problems the software is designed to solve and who the target audience is

Tests

• [x] An automated test suite hooked up to an external service such as Travis-CI or similar

Autorship

• [x] Indicate what authors did

[jonsampedro]

Hello @ibarraespinosa , hope everything is fine too. Thanks, I will be happy to provide any information or clarification that helps with the review process.

[ibarraespinosa]

options(timeout=2000)
#https://stackoverflow.com/questions/35282928/how-do-i-set-a-timeout-for-utilsdownload-file-in-r/35283374
setwd("rfasst/")
library(rfasst)
rpackageutils::download_unpack_zip(
  data_directory = "./inst/extdata",
  url = "https://zenodo.org/record/4763523/files/database_basexdb_5p3_release.zip?download=1")

setwd("..")

file.copy("rfasst/tests/testthat/inst/extdata/queries_rfasst.xml", ".")
download.file(
  url = "https://raw.githubusercontent.com/JGCRI/rfasst/main/tests/testthat/inst/extdata/queries_rfasst.xml", 
  destfile = "queries_rfasst.xml"
)

file.remove("testJOSS.dat")

# To run a function from module 1:
m1_emissions_rescale(db_path =  "rfasst/inst/extdata/",
                     query_path="rfasst/inst/extdata/",
                     db_name = "database_basexdb_5p3_release",
                     prj_name = "testJOSS.dat",
                     scen_name = "Reference_gcam5p3_release",
                     queries ="queries_rfasst.xml",
                     saveOutput = F,
                     map = F)

Here only works with saveOutput = F and map = F

I haven't read al the documentation but it seems that the main output is testJOSS.dat. I will study yourpackage.

[ibarraespinosa]

When I use map = T I get the following:

Error in rmap::map(data = final_df_wide.map, shape = fasstSubset, folder = "output/maps/m1/maps_em",  : 
  unused arguments (mapTitleOn = F, facetCols = 3, legendOutsideSingle = T, legendPosition = c("RIGHT", "bottom"), legendTextSizeO = 0.5, legendTitleSizeO = 0.7)

Also, should I need to create a directory output/maps/m1/?

Thanks

[jonsampedro]

Hello @ibarraespinosa . Just for clarification, the main outputs from the package are the csv files and the maps; the testJOSS.dat is a data file to store the relevant information from the GCAM DB and avoid the need for querying the DB everytime a function is run.

Regarding the saveOutput = F and map = F, as you say, you would need to create that directory for writing the outputs (output/maps/m1/ ), but if you clone the repo, the folders are already there. Considering the number of diverse results that the tool generates, I organized the outputs so that each module produces the files in an individual folder.

I have also realized that there was an update in the downstrem rmap software, so if you activated map=T, there were some errors (as you indicate in the comment). I have made some modifications and the maps can be now generated with no issues, so you may need to the pull the changes, assuming you are testing the main branch, right? I could make a new release, but I thought that it would be more efficient making it after including all your comments... whatever works best!

[ibarraespinosa]

I have updated the package and now everything is running smoothly. Including combinations true and false for maps and output. Great! Comment on the figures. The maps are using the same legend for different pollutants. I think it is better to produce one legend for each pollutan t.

[ibarraespinosa]

I have been following your tutorial and all is goof. map = T takes lots of time after the last image, idkw, but it looks good anyway. I will be running al code. In the meantime, I strongly suggest you to adopt a styling code. There are good examples, such as the guide of ropensci, tidyverse or the guide of google to write R code.

[ibarraespinosa]

i suggest you to add a logical argument to generate annimations

[jonsampedro]

Hello @ibarraespinosa . Thanks for your comments, I will make the suggested changes in the following days and let you know.

Regarding the styling code, I have tried to follow the Google's R Style Guide (https://google.github.io/styleguide/Rguide.html), so that the code includes left-hand assignments and explicitly qualifies namespaces. For the next version, I will add the explicit returns to my functions, and please let me know if you have any additional suggestions.

For the individualized legends and animations, some of these features are defined in the underlying rmap software, but we will work to implement your suggestions.

Thanks!

[ibarraespinosa]

That is good. I see that the software has improved and the paper as well. Regarding the style of code, I strongly suggest add spaces and new lines after commas.

In my opinion, your article and software is good enough to be accepted.

however, a strong suggestion is to include more guidelines to new users so that they can use your software. For instance, I would like to try your software and make some experiments for Brazil, but it can be a bit overwhelming. Anyways, congratulations.

Em sex., 17 de dez. de 2021 às 19:45, jonsampedro @.***> escreveu:

Hello @ibarraespinosa https://github.com/ibarraespinosa . Thanks for your comments, I will make the suggested changes in the following days and let you know.

Regarding the styling code, I have tried to follow the Google's R Style Guide (https://google.github.io/styleguide/Rguide.html), so that the code includes left-hand assignments and explicitly qualifies namespaces. For the next version, I will add the explicit returns to my functions, and please let me know if you have any additional suggestions.

For the individualized legends and animations, some of these features are defined in the underlying rmap software, but we will work to implement your suggestions.

Thanks!

— Reply to this email directly, view it on GitHub https://github.com/JGCRI/rfasst/issues/58#issuecomment-997074291, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGRM74FAPC635EF7O4IP53TURO4SPANCNFSM5HEZ4RGQ . You are receiving this because you were mentioned.Message ID: @.***>

[ibarraespinosa]

As mentioned, the paper and software is good enough and keep updating your amazing software. I will recommend acceptance. Merry Christmas and sorry for the delay.

[jonsampedro]

@ibarraespinosa, thank you for very constructive review. I will continue updating the software in line with your comments and suggestions. Happy Holidays!