Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qualR: An R package to download Sao Paulo and Rio de Janeiro air pollution data #474

Closed
10 of 27 tasks
quishqa opened this issue Oct 15, 2021 · 52 comments
Closed
10 of 27 tasks
Assignees

Comments

@quishqa
Copy link

quishqa commented Oct 15, 2021

Date accepted: 2022-03-08
Submitting Author Name: Mario Gavidia-Calderón
Submitting Author Github Handle: @quishqa
Other Package Authors Github handles: @Schuch666, @mftandra
Repository: https://github.com/quishqa/qualR
Version submitted:0.9.5
Submission type: Standard
Editor: @ldecicco-USGS
Reviewers: @beatrizmilz, @kauedesousa

Due date for @beatrizmilz: 2021-12-01

Due date for @kauedesousa: 2021-12-02
Archive: TBD
Version accepted: TBD


  • Paste the full DESCRIPTION file inside a code block below:
Package: qualR
Title: An R package to download Sao Paulo and Rio de Janeiro air pollution data
Version: 0.9.5
Authors@R: c(
    person(given = "Mario",
           family = "Gavidia-Calderón",
           role = c("aut", "cre"),
           email = "[email protected]",
           comment = c(ORCID = "0000-0002-7371-1116")),
    person(given = "Maria de Fatima",
           family = "Andrade",
           role = c("ctb", "ths"),
           email = "[email protected]",
           comment = c(ORCID = "0000-0001-5351-8311")),
    person(given = "Daniel",
           family = "Schuch",
           role = c("aut","ctb"),
           email = "[email protected]",
           comment = c(ORCID = "0000-0001-5977-4519")))
Description: A package to download information from CETESB QUALAR and
    MonitorAr systems. It contains function to download different parameters, a set of
    criteria pollutants and the most frequent meteorological parameters used in
    air quality data analysis and air quality model evaluation.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.1
Imports: 
    XML,
    httr,
    jsonlite
URL: https://github.com/quishqa/qualR
BugReports: https://github.com/quishqa/qualR/issues
Suggests:
    covr,
    testthat (>= 3.0.0)
Depends: 
    R (>= 3.5.0)
Config/testthat/edition: 3

Scope

  • Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

    • data retrieval
    • data extraction
    • data munging
    • data deposition
    • workflow automation
    • version control
    • citation management and bibliometrics
    • scientific software wrappers
    • field and lab reproducibility tools
    • database software bindings
    • geospatial data
    • text analysis
  • Explain how and why the package falls under these categories (briefly, 1-2 sentences):
    Sao Paulo data is available through CETESB QUALAR System which limits the download to one parameter for one air quality station for one year, Rio de Janeiro is available through data.rio.API which is not so user-friendly. qualR downloads multiple parameters for different air quality stations and produces completed ready-to-use data frames (missing hours are padded out with NA) with a date column in POSIXct type that allows temporal aggregation and compatibility with openair package.

  • Who is the target audience and what are scientific applications of this package?
    Any researchers that work with air quality and weather data of Sao Paulo and Rio de Janeiro: Meteorologists, Epidemiologists, Environmental engineers, postgraduates students that work in air quality modeling and field measurements campaigns, also stakeholders and the community. This packages facilitates the retrival of Sao Paulo and Rio de Janeiro air quality data, facilitates exploratory data analysis, and enhance code reproducibility.

  • Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
    koffing retrieves Sao Paulo State air quality data. qualR offers more functionalities (i.e. allows retrieve many parameters from one air quality station, accepts pollutants abbreviations and air quality station names in functions parameters, etc), and returns complete datasets ready to use (no missing hours, concentration in numeric format). qualR also include Rio de Janeiro city and the location of each air quality stations. qualR is more user friendly and it is actively maintainded.

  • (If applicable) Does your package comply with our guidance around Ethics, Data Privacy and Human Subjects Research?

  • If you made a pre-submission inquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

  • Do you intend for this package to go on CRAN?

  • Do you intend for this package to go on Bioconductor?

  • Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

MEE Options
  • The package is novel and will be of interest to the broad readership of the journal.
  • The manuscript describing the package is no longer than 3000 words.
  • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
  • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
  • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

@ropensci-review-bot
Copy link
Collaborator

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

@ropensci-review-bot
Copy link
Collaborator

🚀

Editor check started

👋

@ropensci-review-bot
Copy link
Collaborator

Checks for qualR (v0.9.5)

git hash: 6ca2ad12

  • ✔️ Package name is available
  • ✔️ has a 'CITATION' file.
  • ✔️ has a 'codemeta.json' file.
  • ✖️ does not have a 'contributing' file.
  • ✔️ uses 'roxygen2'.
  • ✔️ 'DESCRIPTION' has a URL field.
  • ✔️ 'DESCRIPTION' has a BugReports field.
  • ✖️ Package has at no HTML vignettes
  • ✖️ These functions do not have examples: [cetesb_aqs.Rd].
  • ✔️ Package has continuous integration checks.
  • ✔️ Package coverage is 100%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.

Important: All failing checks above must be addressed prior to proceeding

Package License: MIT + file LICENSE


1. 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 12 files) and
  • 2 authors
  • no vignette
  • 4 internal data files
  • 3 imported packages
  • 9 exported functions (median 35 lines of code)
  • 13 non-exported functions in R (median 33 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

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 12 62.6
files_vignettes 0 0.0 TRUE
files_tests 9 87.3
loc_R 495 44.0
loc_tests 61 26.1
num_vignettes 0 0.0 TRUE
data_size_total 4165 65.9
data_size_median 667 61.4
n_fns_r 22 22.5
n_fns_r_exported 9 38.9
n_fns_r_not_exported 13 19.3
n_fns_per_file_r 1 8.3
num_params_per_fn 7 85.6
loc_per_fn_r 34 88.5
loc_per_fn_r_exp 35 70.3
loc_per_fn_r_not_exp 33 89.7
rel_whitespace_R 24 54.8
rel_whitespace_tests 41 75.1
doclines_per_fn_exp 41 51.1
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 34 53.0

1a. Network visualisation

Interactive network visualisation of calls between objects in package can be viewed by clicking here


2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

3a. Continuous Integration Badges

[![travis](github AppVeyor build status Coverage Status)](github AppVeyor build status Coverage Status)


3b. goodpractice results

R CMD check with rcmdcheck

R CMD check generated the following note:

  1. checking data for non-ASCII characters ... NOTE
    Note: found 40 marked UTF-8 strings

R CMD check generated the following check_fail:

  1. rcmdcheck_non_ascii_characters_in_data

Test coverage with covr

Package coverage: 100

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found the following 54 potential issues:

message number of times
Avoid changing the working directory, or restore it in on.exit 2
Lines should not be more than 80 characters. 52


Package Versions

package version
pkgstats 0.0.2.16
pkgcheck 0.0.2.86


Editor-in-Chief Instructions:

Processing may not proceed until the items marked with ✖️ have been resolved.

@quishqa
Copy link
Author

quishqa commented Oct 15, 2021

@ropensci-review-bot help

@ropensci-review-bot
Copy link
Collaborator

Hello @quishqa, here are the things you can ask me to do:


# List all available commands
@ropensci-review-bot help

# Show our Code of Conduct
@ropensci-review-bot code of conduct

@quishqa
Copy link
Author

quishqa commented Oct 20, 2021

Hi!
All items marked as ✖️ have been resolved. We are working on fixing goodpractice checks.

@ldecicco-USGS
Copy link

@ropensci-review-bot check package

@ropensci-review-bot
Copy link
Collaborator

Thanks, about to send the query.

@ropensci-review-bot
Copy link
Collaborator

🚀

Editor check started

👋

@ropensci-review-bot
Copy link
Collaborator

Checks for qualR (v0.9.5)

git hash: ca28ae7b

  • ✔️ Package name is available
  • ✔️ has a 'CITATION' file.
  • ✔️ has a 'codemeta.json' file.
  • ✔️ has a 'contributing' file.
  • ✔️ uses 'roxygen2'.
  • ✔️ 'DESCRIPTION' has a URL field.
  • ✔️ 'DESCRIPTION' has a BugReports field.
  • ✔️ Package has at least one HTML vignette
  • ✔️ All functions have examples.
  • ✔️ Package has continuous integration checks.
  • ✔️ Package coverage is 100%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.

Package License: MIT + file LICENSE


1. 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 12 files) and
  • 2 authors
  • 1 vignette
  • 4 internal data files
  • 3 imported packages
  • 9 exported functions (median 35 lines of code)
  • 13 non-exported functions in R (median 33 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

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 12 62.6
files_vignettes 0 0.0 TRUE
files_tests 9 87.3
loc_R 509 45.0
loc_tests 65 27.3
num_vignettes 1 60.7
data_size_total 4165 65.9
data_size_median 667 61.4
n_fns_r 22 22.5
n_fns_r_exported 9 38.9
n_fns_r_not_exported 13 19.3
n_fns_per_file_r 1 8.3
num_params_per_fn 7 85.6
loc_per_fn_r 34 88.5
loc_per_fn_r_exp 35 70.3
loc_per_fn_r_not_exp 33 89.7
rel_whitespace_R 23 54.8
rel_whitespace_tests 38 75.1
doclines_per_fn_exp 42 52.5
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 34 53.0

1a. Network visualisation

Interactive network visualisation of calls between objects in package can be viewed by clicking here


2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

3a. Continuous Integration Badges

[![travis](github AppVeyor build status Coverage Status)](github AppVeyor build status Coverage Status)


3b. goodpractice results

R CMD check with rcmdcheck

R CMD check generated the following note:

  1. checking data for non-ASCII characters ... NOTE
    Note: found 40 marked UTF-8 strings

R CMD check generated the following check_fail:

  1. rcmdcheck_non_ascii_characters_in_data

Test coverage with covr

Package coverage: 100

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found the following 4 potential issues:

message number of times
Avoid changing the working directory, or restore it in on.exit 4


Package Versions

package version
pkgstats 0.0.2.16
pkgcheck 0.0.2.86


Editor-in-Chief Instructions:

This package is in top shape and may be passed on to a handling editor

@ldecicco-USGS
Copy link

Thanks @quishqa ! The package is looking great. I'll be taking the editor role.

@ropensci-review-bot assign @ldecicco-USGS as editor

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @ldecicco-USGS as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @ldecicco-USGS is now the editor

@ldecicco-USGS
Copy link

Hi @quishqa , I'm still asking around for reviewers (I've reached out to a few that have declined because they are busy). I just wanted to let you know it's taking a bit longer than usual, sorry about that.

@quishqa
Copy link
Author

quishqa commented Nov 9, 2021

Hi @ldecicco-USGS , thanks for the update. We're looking forward to the review.

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @beatrizmilz as reviewer

@ldecicco-USGS
Copy link

@ropensci-review-bot add @beatrizmilz to reviewers

@ropensci-review-bot
Copy link
Collaborator

@beatrizmilz added to the reviewers list. Review due date is 2021-12-01. Thanks @beatrizmilz for accepting to review! Please refer to our reviewer guide.

@ropensci-review-bot
Copy link
Collaborator

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

@ldecicco-USGS
Copy link

@ropensci-review-bot add @kauedesousa to reviewers

@ropensci-review-bot
Copy link
Collaborator

@kauedesousa added to the reviewers list. Review due date is 2021-12-02. Thanks @kauedesousa for accepting to review! Please refer to our reviewer guide.

@kauedesousa
Copy link
Member

Dear @ldecicco-USGS and @quishqa here goes my comments on the package. This is a nice one. Congrats!!

Package Review

  • Briefly describe any working relationship you have (had) with the package authors.
  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (if you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need: clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s): demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions
  • Examples: (that run successfully locally) for all exported functions
  • Community guidelines: including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 5

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

  • Datasets monitor_ar_aqs and monitor_ar_param don't match with the general naming used for the functions. I suggest that you rename these files to match with your naming logic. You may rename like, ArStation and ArParameters
  • Same for the CETESB data sets
  • Part of your functions (for CETESB) need a previous account with CETESB, and you only mention that by the end of your vignette. You need to mention that on the description of the functions CetesbRetrieveMet() and the other related.
  • Your vignette don't run smoothly because you call the package openair with :: without explicitly mentioning it with library(openair). You should call the package with library().
  • In line 114 of /R/CetesbRetrieve.R you start with an if() and then a else with another if(). I think that these statements are very difficult to understand and to fix. Maybe you can use only if()s and stress on the possible outcomes that the call may return to you.
  • You test if the functions are returning the right dimensions of your expected data frame. But you don't test whether the outcome is the correct data. Maybe you can download a small data set from the server and put it in the sysdata.rda and validate it with the output of your functions.
  • Fix the typo in line 16 of file /R/CetesbRetrieveMet.R
  • I recommend to put the paper files in /inst/paper so your main directory is more organized. Same for the png files.
  • Remove README.Rmd and keep only README.md
  • /data-raw/ could be placed inside /data/
  • Remove LICENSE.md
  • In the DESCRIPTION file, put the links for ETESB QUALAR and MonitorAr systems if any inside a <>, like this https://cetesb.sp.gov.br/ar/qualar/ is very likely that CRAN will ask you to do that.
  • Make a file called qualR.R in the \R directory, it makes a nice description of your package. There are some complex versions like this https://github.com/ropensci/rgbif/blob/master/R/rgbif-package.r or simplified versions like this https://github.com/ropensci/chirps/blob/master/R/chirps.R
  • The package is nice and delivers what is promissed. Congrats!!

Session info

─ Session info  🍰  👨‍⚕️  📱   ────────────────────────────────────────────────────────────────────────────────────────
 hash: shortcake, man health worker, mobile phone

 setting  value
 version  R version 4.1.0 (2021-05-18)
 os       macOS 12.0.1
 system   x86_64, darwin17.0
 ui       RStudio
 language (EN)
 collate  en_US.UTF-8
 ctype    en_US.UTF-8
 tz       Europe/Oslo
 date     2021-12-14
 rstudio  1.4.1716 Juliet Rose (desktop)
 pandoc   2.9.1.1 @ /usr/local/bin/pandoc

─ Packages ──────────────────────────────────────────────────────────────────────────────────────────────────────────
 package     * version  date (UTC) lib source
 cachem        1.0.6    2021-08-19 [1] CRAN (R 4.1.0)
 callr         3.7.0    2021-04-20 [1] CRAN (R 4.1.0)
 cli           3.1.0    2021-10-27 [1] CRAN (R 4.1.0)
 crayon        1.4.2    2021-10-29 [1] CRAN (R 4.1.0)
 curl          4.3.2    2021-06-23 [1] CRAN (R 4.1.0)
 desc          1.4.0    2021-09-28 [1] CRAN (R 4.1.0)
 devtools    * 2.4.2    2021-06-07 [1] CRAN (R 4.1.0)
 ellipsis      0.3.2    2021-04-29 [1] CRAN (R 4.1.0)
 fastmap       1.1.0    2021-01-25 [1] CRAN (R 4.1.0)
 fs            1.5.0    2020-07-31 [1] CRAN (R 4.1.0)
 glue          1.4.2    2020-08-27 [1] CRAN (R 4.1.0)
 httr          1.4.2    2020-07-20 [1] CRAN (R 4.1.0)
 jsonlite      1.7.2    2020-12-09 [1] CRAN (R 4.1.0)
 knitr         1.36     2021-09-29 [1] CRAN (R 4.1.0)
 lifecycle     1.0.1    2021-09-24 [1] CRAN (R 4.1.0)
 magrittr      2.0.1    2020-11-17 [1] CRAN (R 4.1.0)
 memoise       2.0.0    2021-01-26 [1] CRAN (R 4.1.0)
 pkgbuild      1.2.0    2020-12-15 [1] CRAN (R 4.1.0)
 pkgload       1.2.3    2021-10-13 [1] CRAN (R 4.1.0)
 prettyunits   1.1.1    2020-01-24 [1] CRAN (R 4.1.0)
 processx      3.5.2    2021-04-30 [1] CRAN (R 4.1.0)
 ps            1.6.0    2021-02-28 [1] CRAN (R 4.1.0)
 purrr         0.3.4    2020-04-17 [1] CRAN (R 4.1.0)
 qualR       * 0.9.5    2021-12-14 [1] Github (quishqa/qualR@dc15bd0)
 R6            2.5.1    2021-08-19 [1] CRAN (R 4.1.0)
 remotes       2.4.1    2021-09-29 [1] CRAN (R 4.1.0)
 rlang         0.4.12   2021-10-18 [1] CRAN (R 4.1.0)
 roxygen2    * 7.1.2    2021-09-08 [1] CRAN (R 4.1.0)
 rprojroot     2.0.2    2020-11-15 [1] CRAN (R 4.1.0)
 rstudioapi    0.13     2020-11-12 [1] CRAN (R 4.1.0)
 sessioninfo   1.2.1    2021-11-02 [1] CRAN (R 4.1.0)
 stringi       1.7.5    2021-10-04 [1] CRAN (R 4.1.0)
 stringr       1.4.0    2019-02-10 [1] CRAN (R 4.1.0)
 testthat    * 3.1.0    2021-10-04 [1] CRAN (R 4.1.0)
 usethis     * 2.1.3    2021-10-27 [1] CRAN (R 4.1.0)
 withr         2.4.2    2021-04-18 [1] CRAN (R 4.1.0)
 xfun          0.27     2021-10-18 [1] CRAN (R 4.1.0)
 XML           3.99-0.8 2021-09-17 [1] CRAN (R 4.1.0)
 xml2          1.3.2    2020-04-23 [1] CRAN (R 4.1.0)

 [1] /Library/Frameworks/R.framework/Versions/4.1/Resources/library

@quishqa
Copy link
Author

quishqa commented Dec 15, 2021

Thanks a lot @kauedesousa for your review will solve the observations in the next days.

@ldecicco-USGS
Copy link

Thanks again @kauedesousa and @beatrizmilz for the great reviews. @quishqa , and updates on the responses?

@quishqa
Copy link
Author

quishqa commented Jan 21, 2022

Hi everybody! We hope to have everything ready before the 28th.

@quishqa
Copy link
Author

quishqa commented Feb 1, 2022

Sorry for the (very) late reply. Thanks a lot to @ldecicco-USGS and to @beatrizmilz and @kauedesousa for the
very exhaustive review. Now the package is definitely better and we also learned a lot
about building packages in R.

Here are the answers to each observation following the commit where we fix them.

Response to @beatrizmilz

  • Consistent naming styles: it would be better to choose a language (English), and a coding style (ex. snake case) and use them in all function names, arguments, and names of columns in the datasets returned.

Response: Now all function names are snake case and the names of columns are the most common in atmospheric sciences.
See ropensci/qualR@10f2dde
And ropensci/qualR@388c779

  • The tests can be improved. Consider also using GitHub Actions to perform an automated check.

Response: We updated the tests to check also number of rows, to check if output is written, to check if variables (a pollutant) mean is inside a range, and to check the class of each columns.
See ropensci/qualR@85cc521
And
ropensci/qualR@036a8e1

  • Files in the root directory: some of the files do not seem fit to be stored there (such as the files related to the paper), and some need to be ignored in the build of the package. I wrote in more detail in the report.

Response: Paper folder moved to inst/paper ropensci/qualR@2d03ff4
Also we updated our .Rbuildignore file see: ropensci/qualR@43df95a

  • The spell-check found some typos that could be corrected.

Response: Corrected in ropensci/qualR@57d14d0

  • I wrote in the report some ideas for the vignettes, but they are only suggestions to make it easier for people to use the data.

Response: We are not used to work with tidyverse. Nervertheless we add one example in the vignette. See ropensci/qualR@4a78c93

  • Modifying directories using setwd() is not something recommended in the book R Packages

Response: By adding the csv_path argument we don't need to used setwd() anymore. See ropensci/qualR@f7f8055
and ropensci/qualR@b0d7665

Other recommendations from full report

  • Add rOpenSci Review badge

Response: Added in ropensci/qualR@e6af4d1

  • The CITATION file exists. There are releases on GitHUb, but I did not find any reference to Zenodo. Reference. There is 0 results searching for qualR on Zenodo: https://zenodo.org/search?page=1&size=20&q=qualR . Read more how to do it and have a DOI for your package on the GitHub documentation.

Response: DOI created in ropensci/qualR@9e4d7f0

  • Code of conduct: in the book, is recommended to use a code of conduct (CoC), and I did not find that information in the README.

Response: Created in ropensci/qualR@bcce5b5

  • R/sysdata.rda should be used to internal datasets.

Response: Yes these dataset is for internal use. It has the name of stations with and without diacritics. Using Tatui is the same as using Tatuí

Response: Corrected in ropensci/qualR@f3d4d2a

  • MonitorArRetrievePol(), MonitorArRetrieveMet() and CetesbRetrieveMetPol() - The names of the columns of the dataset returned are in a good pattern (all in English and snake case). The column classes I think it would be better if the numeric variables (such as the air parameters) returned numeric/double (now some of them are lgl because there is only NA’s).

Response: Columns names from returned dataset standarized. See ropensci/qualR@388c779 .
Now numeric variables are returned as numeric even when there is only NA. See ropensci/qualR@cb808be

  • Console messages - It is not recommended to use cat() to communicate with the user of the functions

Response: Now we use message(). See ropensci/qualR@0c90cfd

  • IDEA - When the function is storing the csv with to_csv = TRUE, it would be nice to have an argument to write the path where the csv will be saved.

Response: Accepted in ropensci/qualR@f7f8055. It helped to avoid using setwd() on tests.

  • IDEA - I think it would be nice to be able to search more than one station in the functions.

Response: We put a way to do it in the README section in https://github.com/quishqa/qualR#a-variable-from-all-cetesb-aqs

  • All the tests passed. There are only a few tests. I recommend the authors to write more expectations with expect_*(), such as: expect the class of the result, the class of the columns in the dataset.

Response: Now we evaluate classes. See ropensci/qualR@85cc521

  • There is a test are not testing anything related to the package: In test-CetesbRetrieveParam.R, the only expectation is expect_equal(2 * 2, 4).

Response: Corrected in ropensci/qualR@85cc521

  • There is no GitHub action to check the package consistently.

Response: Added in ropensci/qualR@60592c9

Response to @kauedesousa

  • Datasets monitor_ar_aqs and monitor_ar_param don't match with the general naming used for the functions. I suggest that you rename these files to match with your naming logic. You may rename like, ArStation and ArParameters

Response: Now we changed all function names using snake case. See ropensci/qualR@10f2dde

  • Same for the CETESB data sets

Response: Now returned datasets have the most common names in atmospheric sciences. See ropensci/qualR@388c779

  • Your vignette don't run smoothly because you call the package openair with :: without explicitly mentioning it with library(openair). You should call the package with library().

Response: Added in ropensci/qualR@f3d4d2a

  • In line 114 of /R/CetesbRetrieve.R you start with an if() and then a else with another if(). I think that these statements are very difficult to understand and to fix. Maybe you can use only if()s and stress on the possible outcomes that the call may return to you.

Response: Fixed in ropensci/qualR@985714f

  • You test if the functions are returning the right dimensions of your expected data frame. But you don't test whether the outcome is the correct data. Maybe you can download a small data set from the server and put it in the sysdata.rda and validate it with the output of your functions.

Response: We updated the tests to check also number of rows, to check if output is written, to check if variables (a pollutant) mean is inside a range, and to check the class of each columns.
See ropensci/qualR@85cc521
And
ropensci/qualR@036a8e1

  • Fix the typo in line 16 of file /R/CetesbRetrieveMet.R

Response: Corrected in ropensci/qualR@57d14d0

  • I recommend to put the paper files in /inst/paper so your main directory is more organized. Same for the png files.

Response: Moved in ropensci/qualR@2d03ff4

  • Remove README.Rmd and keep only README.md

Response: Removed in ropensci/qualR@b32c6ff

  • /data-raw/ could be placed inside /data/

Response: That folder was created when using the function usethis::use_data_raw() during the package development.

  • Remove LICENSE.md

Response: We still have doubts about which one we should remove LICENSE.md has more information.

  • In the DESCRIPTION file, put the links for ETESB QUALAR and MonitorAr systems if any inside a <>, like this https://cetesb.sp.gov.br/ar/qualar/ is very likely that CRAN will ask you to do that.

Response: Added in ropensci/qualR@406cfc6

  • Make a file called qualR.R in the \R directory, it makes a nice description of your package.

Response: Created in ropensci/qualR@30b61db

@beatrizmilz
Copy link

Hi @quishqa ! I'm happy to know that the review was usefull. Congratulations for the extensive work!
Seems like the website was not fully updated. Some of the pages on the reference are returning error 404, in the case when the function names were changed: https://quishqa.github.io/qualR/reference/cetesb_retrieve_param.html
Probably using pkgdown::build_site() would fix it :)

@quishqa
Copy link
Author

quishqa commented Feb 1, 2022

Thanks for noticing this. It's weird because the manual from the datasets are OK but only the functions (which the names were changed) are presenting this error. I already do pkgdown::build_site() . Locally it works but on the deployed web it does not work. I'm going to do a little more research.

@quishqa
Copy link
Author

quishqa commented Feb 4, 2022

Dear all, the returning 404 error was fixed in ropensci/qualR@6e4ce0d

@beatrizmilz
Copy link

Hi Mario! I checked here and the website is working :) Congratulations.

Congratulations for the extensive work.

About the vignette, I'm glad that you added an example with ggplot2! ggplot2 is an great package to create visualizations, and enables us to create elegant graphs with the data that qualR returns.

About the other suggestions and comments that I wrote, seems like they all have been considered. devtools::check() passed as well.

Tests

I still think that the tests could be improved. I wrote some comments and I hope they can be helpfull if you want to expand the testing in the package:

  • As @kauedesousa have said, is good to test whether the outcome is the correct data. One function that you can try is expect_snapshot(): in the first time you use it, it will save the expected outcome and will check in the future if the results are still the expected, comparing with what was saved.

  • It seems to me that not all functions have tests consistently: some of the functions have more extensive tests and others tests only the number of rows and columns for example. Having good tests in all functions helps to find bugs in the future.

  • Also, I think is a good idea to improve the name of the tests, because if it breaks, is easier to understand which is in the check log (some of them are test_that("multiplication works", {). As said in R Packages book, "The main thing is that the message associated with the test should be informative so that you can quickly narrow down the source of the problem." (reference). But this should be a quick fix.

  • I really like this chapter about tests in the R Packages book!


Again, congratulations Mario!

@quishqa
Copy link
Author

quishqa commented Feb 15, 2022

Thanks again for the feedback! We updated the test for all the functions we evaluated dims. values and classes,
see ropensci/qualR@29eb915
and ropensci/qualR@3d3aaf3

Also, we updated the test messages in ropensci/qualR@0a6b148

@kauedesousa
Copy link
Member

I think the updates address our comments. It looks fine with me.

@quishqa
Copy link
Author

quishqa commented Mar 4, 2022

Dear @ldecicco-USGS , @beatrizmilz , and @kauedesousa , What is the status of the review? do you need any extra information or correction?

@ldecicco-USGS
Copy link

Sorry about that. Everything looks great! I'm heading out of town for the weekend, but will kick off the approval process on Monday assuming @beatrizmilz and @kauedesousa don't have any objections.

@kauedesousa
Copy link
Member

No objection. I recommend the approval. I just wonder why you didn't submit it with a paper for JOSS. Anyways, thanks for opportunity to review the R package.

@beatrizmilz
Copy link

I also recommend the approval!

@ldecicco-USGS
Copy link

@ropensci-review-bot approve qualR

@ropensci-review-bot
Copy link
Collaborator

Approved! Thanks @quishqa for submitting and @beatrizmilz, @kauedesousa for your reviews! 😁

To-dos:

  • Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so.
  • After transfer write a comment @ropensci-review-bot finalize transfer of <package-name> where <package-name> is the repo/package name. This will give you admin access back.
  • Fix all links to the GitHub repo to point to the repo under the ropensci organization.
  • Delete your current code of conduct file if you had one since rOpenSci's default one will apply, see https://devguide.ropensci.org/collaboration.html#coc-file
  • If you already had a pkgdown website and are ok relying only on rOpenSci central docs building and branding,
    • deactivate the automatic deployment you might have set up
    • remove styling tweaks from your pkgdown config but keep that config file
    • replace the whole current pkgdown website with a redirecting page
    • replace your package docs URL with https://docs.ropensci.org/package_name
    • In addition, in your DESCRIPTION file, include the docs link in the URL field alongside the link to the GitHub repository, e.g.: URL: https://docs.ropensci.org/foobar (website) https://github.com/ropensci/foobar
  • Fix any links in badges for CI and coverage to point to the new repository URL.
  • Increment the package version to reflect the changes you made during review. In NEWS.md, add a heading for the new version and one bullet for each user-facing change, and each developer-facing change that you think is relevant.
  • We're starting to roll out software metadata files to all rOpenSci packages via the Codemeta initiative, see https://docs.ropensci.org/codemetar/ for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.
  • You can add this installation method to your package README install.packages("<package-name>", repos = "https://ropensci.r-universe.dev") thanks to R-universe.

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. She 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.

@maelle
Copy link
Member

maelle commented Mar 18, 2022

@quishqa I've just noticed this hasn't been transferred yet, do you need any help?

@quishqa
Copy link
Author

quishqa commented Mar 18, 2022

Hi @maelle, actually we do. To do the first task we only need to follow this, right? And we don't know where we need to do the @ropensci-review-bot finalize transfer of <package-name> comment?

Thanks!

@maelle
Copy link
Member

maelle commented Mar 18, 2022

Yes regarding the repo transfer!

Then the comment @ropensci-review-bot finalize transfer of qualR needs to be put here in an issue comment.

Thank you and have a good week-end!

@quishqa
Copy link
Author

quishqa commented Mar 18, 2022

@ropensci-review-bot finalize transfer of qualR

@ropensci-review-bot
Copy link
Collaborator

Transfer completed. The qualR team is now owner of the repository

@quishqa
Copy link
Author

quishqa commented Mar 25, 2022

Hi @beatrizmilz and @kauedesousa , is it ok for you if we can add you to the DESCRIPTION file? We'll feel very honored.

@kauedesousa
Copy link
Member

that is ok with me @quishqa

@beatrizmilz
Copy link

It is ok with me too @quishqa !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants