mitchellmanware
commented
3 months ago The unit test for the calc_lagged function with geom = TRUE exhibits very strange behavior. The testthat::expect_no_error unit tests run without error, but when narr_lag_geom_setcols is called in the specific testthat::expect_true tests, it cannot be found.
This error was also occurring on a previous PR (https://github.com/NIEHS/amadeus/pull/105/commits/39f9b336e2ec23883035ec17734af728d8c38a01 and https://github.com/NIEHS/amadeus/pull/105/commits/3d17f5e3ed1f699b098b2ab1ec1405810bdca4bf) without reason.
The tests run and pass locally, so I am not sure what is causing this error. I removed the specific tests for now.
sigmafelix
Updates from CRAN Submission amadeus 1.0.2
Italicized are the comments from the reviewer, and bullet points are my changes to address comment the comment or an explanation of why it was not adopted.
Please reduce the length of the title to less than 65 characters.
"Accessing and Analyzing Large Scale Environmental Data in R"
New title matches AGU abstract title.
https://github.com/NIEHS/amadeus/commit/652903c4b011b27be49ec002ddc2397020f2101d
If there are references describing the methods in your package, please add these in the description field of your DESCRIPTION file in the form authors (year)
authors (year, ISBN:...)
or if those are not available: <https:...>
with no space after 'doi:', 'https:' and angle brackets for
auto-linking. (If you want to add a title as well please put it in
quotes: "Title”)
DESCRIPTIONfile updated with citation forcalc_sedcfunction, which applies published statistical methods (https://pubs.acs.org/doi/10.1021/es203152a)https://github.com/NIEHS/amadeus/commit/cd105e957dce8b39fdebf19ee48a23ec9e17f7b8
Please always add all authors, contributors and copyright holders in the Authors@R field with the appropriate roles. From CRAN policies you agreed to: "The ownership of copyright and intellectual property rights of all components of the package must be clear and unambiguous (including from the authors specification in the DESCRIPTION file). Where code is copied (or derived) from the work of others (including from R itself), care must be taken that any copyright/license statements are preserved and authorship is not misrepresented. Preferably, an ‘Authors@R’ would be used with ‘ctb’ roles for the authors of such code. Alternatively, the ‘Author’ field should list these authors as contributors. Where copyrights are held by an entity other than the package authors, this should preferably be indicated via ‘cph’ roles in the ‘Authors@R’ field, or using a ‘Copyright’ field (if necessary referring to an inst/COPYRIGHTS file)." e.g.: -> "Spatiotemporal Exposures and Toxicology Group" in your LICENSE file Please explain in the submission comments what you did about this issue.
“Spatiotemporal Exposures and Toxicology Group” has been added to the Authors@R field as a copyright holder.
This change reflects the
COPYRIGHT HOLDERsection of theLICENSEfile.https://github.com/NIEHS/amadeus/commit/cd105e957dce8b39fdebf19ee48a23ec9e17f7b8
_Please add \value to .Rd files regarding exported methods and explain the functions results in the documentation. Please write about the structure of the output (class) and also what the output means. (If a function does not return a value, please document that too, e.g. \value{No return value, called for side effects} or similar) Missing Rd-tags: calc_check_time.Rd: \value calc_message.Rd: \value check_for_null_parameters.Rd: \value check_mysf.Rd: \value check_mysftime.Rd: \value download_data.Rd: \value download_permit.Rd: \value download_remove_command.Rd: \value download_remove_zips.Rd: \value download_run.Rd: \value download_sink.Rd: \value download_unzip.Rd: \value test_downloadfunctions.Rd: \value
\value tags updated to describe output/processes even when function has no return into the R environment.
https://github.com/NIEHS/amadeus/commit/cd105e957dce8b39fdebf19ee48a23ec9e17f7b8
\dontrun{} should only be used if the example really cannot be executed (e.g. because of missing additional software, missing API keys, ...) by the user. That's why wrapping examples in \dontrun{} adds the comment ("# Not run:") as a warning for the user. Does not seem always necessary. Please replace \dontrun with \donttest. Please unwrap the examples if they are executable in < 5 sec, or replace dontrun{} with \donttest{}. Please put functions which download data in \donttest{}.
Download function examples have
download = FALSE, so there is no data downloaded.https://github.com/NIEHS/amadeus/commit/cd105e957dce8b39fdebf19ee48a23ec9e17f7b8
Examples for the
process_*()andcalc_*()functions remain in the /donttest brackets.These examples require spatial data to run without errors, and including the data (by including tests/testdata in build [currently on .Rbuildignore] or moving data to inst/extdata) inflates the package size substantially.
A small spatial subset (~3 x 3 grid cells) are used for tests, but still make the package very large (>338 Mb)
Below is a log snapshot from a build check with example data moved to inst/extdata.
You write information messages to the console that cannot be easily suppressed. It is more R like to generate objects that can be used to extract the information a user is interested in, and then print() that object. Instead of print()/cat() rather use message()/warning() or if(verbose)cat(..) (or maybe stop()) if you really have to write text to the console. (except for print, summary, interactive functions) -> R/download.R
The series of
download_*()functionscat()command line commands to a .txt file on the user’s machine in order to download data files from URLs.cat()is only used for writing these commands to text file.The
message()andstop()commands are used to provide the user with progress and error messages, respectively.Please ensure that your functions do not write by default or in your examples/vignettes/tests in the user's home filespace (including the package directory and getwd()). This is not allowed by CRAN policies. Please omit any default path in writing functions. In your examples/vignettes/tests you can write to tempdir(). -> download-functions and their examples
Download function examples have been updated to utilize
tempdir().https://github.com/NIEHS/amadeus/commit/cd105e957dce8b39fdebf19ee48a23ec9e17f7b8
The workflow.Rmd did not write to the specified files as the chunks were set to
eval = FALSE, but they have also been updated to use thetempdir()writing location.Tests have been updated to use
tempdir()instead of manually creating/writing/removing directories within the “testdata” folder.https://github.com/NIEHS/amadeus/commit/652903c4b011b27be49ec002ddc2397020f2101d
Functionality Fixes
download_runBeta testing by Umit Tokac revealed that the
download_*()functions were not downloading any data files when run on Windows OSProblem was related to file type and command executer that were default in
download_runfor all OSNew steps
download_remove_commandNew steps 4 and 6 conslidate 10+ lines of code from each
download_*()function into thedownload_runcommand while retaining all functionality and tests https://github.com/NIEHS/amadeus/blob/d5683a689df02062276ab802a1285dbfecdc6cc1/R/download_auxiliary.R#L110Function successfully downloads data files on Windows OS
download_remove_zipspak::pak("NIEHS/amadeus@cran-0801")to loadamadeusin order to avoid accidental data deletion @kyle-messier @eva0marques @eva0marquesOther
@referencessection for cleaner references in pkgdown websitecalc_lagged(geom = TRUE)(did not work in last pull request for unknown reason)