ropensci / software-review

rOpenSci Software Peer Review.
292 stars 104 forks source link

Submission: ReLTER #485

Closed oggioniale closed 2 years ago

oggioniale commented 3 years ago

Date accepted: 2022-04-27 Reviewers: Submitting Author Name: Alessandro Oggioni Submitting Author Github Handle: !--author1-->@oggioniale<!--end-author1-- Other Package Authors Github handles: (comma separated, delete if none) @micha-silver, @ptagliolato Repository: https://github.com/oggioniale/ReLTER Version submitted: 1.0.0 Submission type: Standard Editor: !--editor-->@maurolepore<!--end-editor-- Reviewers: @WillOnGit, @allisonhorst

Due date for @allisonhorst: 2022-03-10 Due date for @WillOnGit: 2022-03-10

Archive: TBD Version accepted: TBD Language: en


Package: ReLTER
Type: Package
Title: An interface to the eLTER for the R statistical programming environment
Version: 1.0.0
Date: 2021-11-15
Authors@R: c(
        person("Alessandro", "Oggioni", email = "oggioni.a@irea.cnr.it", 
          role = c("aut", "cre", "fnd"),
          comment = c(ORCID = "0000-0002-7997-219X")
        ),
        person("Micha", "Silver", email = "silverm@post.bgu.ac.il",
          role = c("aut", "ctb"),
          comment = c(ORCID = "0000-0002-1128-1325",
          " Micha Silver is a coauthor of functions get_ilter_generalinfo and
          get_site_info for category Boundaries, he develop get_site_ODS
          function and he also is a reviewer of English wording for all
          functions.")
        ),
        person("Paolo", "Tagliolato", email = "tagliolato.a@irea.cnr.it",
          role = c("aut", "ctb"),
          comment = c(ORCID = "0000-0002-0261-313X",
          " Paolo Tagliolato is a coauthor of package, he wrote the function
          do_Q(), on which some ReLTER internal functions are based")
        )
      )
Maintainer: The package maintainer Alessandro Oggioni, phD (2020)
    <oggioniale@gmail.com>
Description: ReLTER is an R package that: provides access to DEIMS-SDR
    (https://deims.org/), allows interact with software implemented by eLTER
    Research Infrastructure (RI) and improves the data/information shared by
    them.
    ReLTER is a R package devoted to access, interact and improve the
    information and the data shared by Long Term Ecological Research (LTER)
    network. This package is born within eLTER H2020 major project that will
    help advance the development of European Long-Term Ecosystem Research
    Infrastructures (eLTER RI - https://elter-ri.eu).
    The ReLTER package functions in particular allow to:
    - retrive the information about entities (e.g. sites, datasets, and
    activities) shared by DEIMS-SDR (see e.g. get_site_info function);
    - interact with the [ODSEurope](maps.opendatascience.eu) managed by members
    of the [Geo-harmonizer](https://opendatascience.eu/geoharmonizer-project/)
    project starting with the dataset shared by [DEIMS-SDR](https://deims.org/)
    (see e.g. [get_site_ODS](https://oggioniale.github.io/ReLTER/reference/get_site_ODS.html)
    function);
    - use the site informations for download data from other platforms (see
    e.g. get_site_ODS function);
    - improve the quality of the dataset (see e.g. get_id_worms).
    Functions currently implemented are derived from the discussion of the
    needs declared by eLTER users community.
    The ReLTER package shall definitely follow the progress of eLTER-RI
    infrastructure and evolving with the improvements and develop of
    new tools.
License: GPL-3
Encoding: UTF-8
LazyData: true
Depends: R (>= 3.5.0)
Imports:
    dplyr (>= 1.0.0),
    jsonlite (>= 1.7.2),
    sf (>= 0.9-5),
    tibble (>= 3.0.5),
    httr (>= 1.4.2),
    taxize (>= 0.9.97),
    sp (>= 1.4-2),
    rgeos (>= 0.5-3),
    rosm (>= 0.2.5),
    raster (>= 3.3-13),
    terra (>= 1.4-11),
    tmap (>= 3.1),
    grid (>= 3.5.1),
    leaflet (>= 2.0.3),
    qrencoder (>= 0.1.0),
    waffle (>= 0.7.0),
    dtplyr,
    ISOcodes (>= 2021.02.24),
    RColorBrewer (>= 1.1-2),
    ggforce (>= 0.3.2),
    ggplot2 (>= 3.3.5),
    jqr (>= 1.2.0),
    mapview (>= 2.9.0),
    qrcode (>= 0.1.3),
    scales (>= 1.1.1),
    xml2 (>= 1.3.2),
    magrittr (>= 2.0.1),
    stats (>= 4.0.3)
Suggests: 
    testthat (>= 3.0.0),
    knitr,
    rmarkdown
VignetteBuilder: knitr
URL: https://github.com/oggioniale/ReLTER
BugReports: https://github.com/oggioniale/ReLTER/issues
Config/testthat/edition: 3
RoxygenNote: 7.1.2
Language: en-GB

Scope

ReLTER package devoted to access, interact and improve the information and the data shared on World Wide Web by Long Term Ecological Research (LTER) network. Most of this data has geographic characteristics and the package provides some functions to accessing and manipulating that.

The target audience are primarily the site managers, researchers and operators of International Long Term Ecological Research (ILTER) network of networks. Currently, ILTER encompasses 39 national networks which together operate more than 600 sites and more than 1300 involved persons. ILTER's focus is on long-term data series, site-based research and monitoring. ReLTER access, interact and improve the information and the data shared by European Long-Term Ecosystem Research Infrastructures (eLTER RI) trough web services and APIs. The target audience in perspective, since ReLTER relates to the data and information released by the largest network dedicated to ecological research, will be all users who need to work on ecology and environmental scientific area.

To the our knowledge there aren't currently R packages that accomplish the same thing of ReLTER. MetaEgress and rOpenSci popler packages are developed for same community but none of them have aims comparable with ReLTER.

N/A

N/A

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

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](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/journal-resources/policy-on-publishing-code.html)) - (*Scope: Do consider MEE's [Aims and Scope](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/aims-and-scope/read-full-aims-and-scope.html) 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

maurolepore commented 2 years ago

@oggioniale please note:

The bot fails to build the package (see above "2. goodpractice and other checks"):

Error : Build failed, unknown error, standard output:

checking for file ‘ReLTER/DESCRIPTION’ ... OK
preparing ‘ReLTER’:
checking DESCRIPTION meta-information ... OK
installing the package to build vignettes
-----------------------------------
ERROR: dependencies ‘jqr’, ‘xslt’ are not available for package ‘ReLTER’
removing ‘/tmp/Rtmp1CudbO/Rinst63e435db54b2/ReLTER’
-----------------------------------
ERROR: package installation failed

And the project on rstudio cloud shows many tests fail with an error similar to this one:

Error (test-get_ilter_generalinfo.R:23:3): Output of ILTER general info function constructs 'tibble' as
          expected
Error in `get_id(deimsid, "sites")`: deimsid '5de9ae4c-c705-4cd5-b55a-2195c936f7fb' cannot be used in test mode with resource = 'sites'.
Backtrace:
 1. ReLTER::get_ilter_generalinfo(country_name = NA, site_name = NA) test-get_ilter_generalinfo.R:23:2
 2. base::lapply(...) /cloud/project/R/get_ilter_generalinfo.R:85:2
 3. ReLTER:::FUN(X[[i]], ...)
 4. ReLTER::get_id(deimsid, "sites") /cloud/project/R/get_site_info.R:48:2

The rstudio.cloud project may be a good platform for you to try explore why the package works fine in your own computers and on GitHub Actions but not on other environments.

mpadge commented 2 years ago

@maurolepore @oggioniale This could be due to the RStudio system deps server being intermittently unreliable, and our system then failing to install the system requirements for jqr and xslt. I'll re-deploy the check system with them pre-installed and run the checks again. Sorry for any inconvenience!

oggioniale commented 2 years ago

@maurolepore I'm disappointed on the situation. Before to write the comment I checked all on local OS, otherwise I would never have told you that everything worked.

So

  1. about the dependencies ‘jqr’, ‘xslt’ where the bot fails? Because the status of the bot in my GitHub profile is "Success";
  2. I ran the rcmdcheck::rcmdcheck(".")in the project on rstudio cloud and the output is
    ── R CMD check results ─────────────────────────────── ReLTER 1.0.0 ────
    Duration: 5m 53.5s
    0 errors ✓ | 0 warnings ✓ | 0 notes ✓ 

if I ran goodpractice::gp(path = ".") in the project on rstudio cloud the output is

── GP ReLTER ───────────────────────────────────────────────
It is good practice to
  ✖ write unit tests for all functions, and all package code in general. 39% of code lines are covered by test cases.

    R/get_activity_info.R:39:NA
    R/get_activity_info.R:40:NA
    R/get_activity_info.R:41:NA
    R/get_activity_info.R:56:NA
    R/get_activity_info.R:57:NA
    ... and 1015 more lines
─────────────────────────────────────────────────────────

Nothing about the problem of the dependencies and of the errors. Please, Let me know how to reproduce the errors, the issues or the fails.

oggioniale commented 2 years ago

@maurolepore I added to che check-standard.yaml the installation of library libxslt1-dev.

I also changed the R version in DESCRIPTION.

mpadge commented 2 years ago

@ropensci-review-bot check package

ropensci-review-bot commented 2 years ago

Thanks, about to send the query.

ropensci-review-bot commented 2 years ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 2 years ago

Checks for ReLTER (v1.0.0)

git hash: 4b82829d

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

Package License: GPL (>= 3)


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 36 files) and - 4 authors - 1 vignette - no internal data file - 29 imported packages - 33 exported functions (median 28 lines of code) - 35 non-exported functions in R (median 44 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 | 36| 92.1| | |files_vignettes | 1| 68.4| | |files_tests | 27| 97.6| | |loc_R | 1926| 83.5| | |loc_vignettes | 48| 8.2| | |loc_tests | 1345| 90.7| | |num_vignettes | 1| 64.8| | |n_fns_r | 68| 65.9| | |n_fns_r_exported | 33| 80.4| | |n_fns_r_not_exported | 35| 57.4| | |n_fns_per_file_r | 1| 0.2|TRUE | |num_params_per_fn | 1| 1.6|TRUE | |loc_per_fn_r | 38| 83.7| | |loc_per_fn_r_exp | 28| 60.2| | |loc_per_fn_r_not_exp | 44| 88.0| | |rel_whitespace_R | 1| 14.0| | |rel_whitespace_vignettes | 46| 15.1| | |rel_whitespace_tests | 13| 83.0| | |doclines_per_fn_exp | 24| 21.5| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 38| 60.5| | ---

1a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

#### 3a. Continuous Integration Badges [![R-CMD-check](https://github.com/oggioniale/ReLTER/workflows/R-CMD-check/badge.svg)](https://github.com/oggioniale/ReLTER/actions) **GitHub Workflow Results** |name |conclusion |sha |date | |:-------------------------------------|:----------|:------|:----------| |.github/workflows/check-standard.yaml |failure |7669f7 |2021-12-13 | |pages build and deployment |success |4b8282 |2021-12-22 | |R-CMD-check |failure |4b8282 |2021-12-22 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) R CMD check generated the following error: 1. checking tests ... Running ‘testthat.R’ ERROR Running the tests in ‘tests/testthat.R’ failed. Last 13 lines of output: 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_network_research_topics.R:25:3): Output of network research topics function constructs ‘tibble’ as expected ── Error in `get_id(deimsid, "sites")`: deimsid 'fcc28bb3-551a-4396-819c-0589abc6be6f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_network_research_topics(networkDEIMSID = TESTURLNetwork) test-get_network_research_topics.R:25:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") [ FAIL 7 | WARN 0 | SKIP 1 | PASS 273 ] Error: Test failures Execution halted R CMD check generated the following test_fail: 1. > library(testthat) > library(ReLTER) ReLTER is specially drafted for the LTER community. To contribute to the improvement of this package, join the group of developers (https://github.com/oggioniale/ReLTER). If you use this package, please cite as: Alessandro Oggioni, Micha Silver, Luigi Ranghetti & Paolo Tagliolato. (2021) oggioniale/ReLTER: ReLTER v1.0.0 (1.0.0). Zenodo. https://doi.org/10.5281/zenodo.5576813 Type 'citation(package = 'ReLTER')' on how to cite R packages in publications. > > test_check("ReLTER") ---- Test get_activity_info() ---- Error: GET https://deims.org/api/activities/8786fc6d-5d70-495c-b901-42f480182845 Request failed [ERROR]. Retrying in 1.1 seconds... Error: GET https://deims.org/api/activities/8786fc6d-5d70-495c-b901-42f480182845 Request failed [ERROR]. Retrying in 1 seconds... ---- Test get_dataset_info() ---- Error: GET https://deims.org/api/datasets/38d604ef-decb-4d67-8ac3-cc843d10d3ef Request failed [ERROR]. Retrying in 1.1 seconds... Error: GET https://deims.org/api/datasets/38d604ef-decb-4d67-8ac3-cc843d10d3ef Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found. Please check again the Dataset.iD ---- NULL ---- The requested page could not be found. Please check again the Dataset.iD ---- NULL ---- Test get_ilter_envcharacts() ---- Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1 seconds... ---- The `sitesNum` value must be a double (e.g. 10, 24, etc.). Please check again the value of `sitesNum`. ---- ---- Test get_ilter_generalinfo() ---- Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1.5 seconds... ---- Test get_ilter_parameters() ---- Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 2.4 seconds... ---- The `sitesNum` value must be a double (e.g. 10, 24, etc.). Please check again the value of `sitesNum`. ---- ---- Test get_ilter_research_topics() ---- Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1.6 seconds... Error: GET https://deims.org/api/sites/3e97f1ff-c3ad-4d19-91f9-5ee3485878cd Request failed [ERROR]. Retrying in 1 seconds... ---- The `sitesNum` value must be a double (e.g. 10, 24, etc.). Please check again the value of `sitesNum`. ---- ---- Test get_network_envcharacts() ---- Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 1.3 seconds... Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 2.3 seconds... ---- The requested page could not be found. Please check again the Network.iD ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- Test get_network_parameters() ---- Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 1.8 seconds... Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 2 seconds... ---- The requested page could not be found. Please check again the Network.iD ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- Test get_network_related_resources() ---- Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 1.6 seconds... Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found. Please check again the Network.iD ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- Test get_network_research_topics() ---- Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 1.1 seconds... Error: GET https://deims.org/api/sites/fcc28bb3-551a-4396-819c-0589abc6be6f Request failed [ERROR]. Retrying in 3.8 seconds... ---- The requested page could not be found. Please check again the Network.iD ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- Test get_network_sites() ---- Error: GET https://deims.org/api/sites?network=e0f680c2-22b1-4424-bf54-58aa9b7476a0 Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites?network=e0f680c2-22b1-4424-bf54-58aa9b7476a0 Request failed [ERROR]. Retrying in 1.6 seconds... Error: GET https://deims.org/api/sites?network=e0f680c2-22b1-4424-bf54-58aa9b7476a0 Request failed [ERROR]. Retrying in 4.8 seconds... Error: GET https://deims.org/api/sites?network=e0f680c2-22b1-4424-bf54-58aa9b7476a0 Request failed [ERROR]. Retrying in 9.5 seconds... ---- The requested page could not be found. Please check the Network ID ---- ---- The requested page could not be found. Please check the Network ID ---- ---- Test get_site_affiliations() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 2.6 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test get_site_boundaries() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found.Please check the DEIMS ID ---- ---- The requested page could not be found.Please check the DEIMS ID ---- ---- Test get_site_contact() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1.5 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test get_site_envcharacts() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1.7 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 2.7 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test get_site_general() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test get_site_infrastructure() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1.8 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 2.3 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test get_site_ODS() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1.5 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 2.3 seconds... ---- This site does not have boundaries uploaded to DEIMS-SDR. Please verify in the site page: https://deims.org/8a313716-ceed-4f41-8b0b-a8197bfc304a ---- [1] "No boundary for requested DEIMS site." ---- Test get_site_parameters() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found. Please check the DEIMS ID ---- ---- The requested page could not be found. Please check the DEIMS ID ---- ---- Test get_site_related_resources() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 2.2 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test get_site_research_topics() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1.3 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1.5 seconds... ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test produce_network_points_map() ---- Error: GET https://deims.org/api/sites?network=e904354a-f3a0-40ce-a9b5-61741f66c824 Request failed [ERROR]. Retrying in 1.8 seconds... Error: GET https://deims.org/api/sites?network=e904354a-f3a0-40ce-a9b5-61741f66c824 Request failed [ERROR]. Retrying in 2.8 seconds... Error: GET https://deims.org/api/sites?network=e904354a-f3a0-40ce-a9b5-61741f66c824 Request failed [ERROR]. Retrying in 1.9 seconds... Error: GET https://deims.org/api/sites?network=e904354a-f3a0-40ce-a9b5-61741f66c824 Request failed [ERROR]. Retrying in 5.1 seconds... trying URL 'https://biogeo.ucdavis.edu/data/gadm3.6/Rsp/gadm36_DEU_0_sp.rds' Content type 'text/html; charset=iso-8859-1' length 686950 bytes (670 KB) ================================================== downloaded 670 KB ---- The requested page could not be found. Please check again the Network.iD ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- The map of site cannot be created. Please check again the Country code. Compare the code provided with the list of code in https://en.wikipedia.org/wiki/ISO_3166 ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- The requested page could not be found. Please check again the Network.iD ---- ---- Test produce_site_parameters_pie() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found. Please check the DEIMS ID ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check the DEIMS ID ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test produce_site_parameters_waffle() ---- Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... Error: GET https://deims.org/api/sites/f30007c4-8a6e-4f11-ab87-569db54638fe Request failed [ERROR]. Retrying in 1 seconds... ---- The requested page could not be found. Please check the DEIMS ID ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- The requested page could not be found. Please check the DEIMS ID ---- ---- The requested page could not be found. Please check again the DEIMS.iD ---- ---- Test produce_site_qrcode() ---- ══ Skipped tests ═══════════════════════════════════════════════════════════════ • empty test (1) ══ Failed tests ════════════════════════════════════════════════════════════════ ── Error (test-get_ilter_generalinfo.R:23:3): Output of ILTER general info function constructs 'tibble' as expected ── Error in `get_id(deimsid, "sites")`: deimsid '5de9ae4c-c705-4cd5-b55a-2195c936f7fb' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_ilter_generalinfo(country_name = NA, site_name = NA) test-get_ilter_generalinfo.R:23:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_ilter_generalinfo.R:55:3): Wrong input (not a character) of 'site_name', but correct input of 'country_name'', constructs an empty tibble ── Error in `get_id(deimsid, "sites")`: deimsid '11aff5a6-d464-40fa-b218-fcad8f57c81f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_ilter_generalinfo(country_name = "Austri", site_name = "123") test-get_ilter_generalinfo.R:55:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_ilter_generalinfo.R:86:3): Output of function constructs 'sf' with valid geometries ── Error in `get_id(deimsid, "sites")`: deimsid '11aff5a6-d464-40fa-b218-fcad8f57c81f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_ilter_generalinfo(country_name = "Austri", site_name = " Eisen") test-get_ilter_generalinfo.R:86:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_network_envcharacts.R:25:3): Output of network environmental characteristics function constructs 'tibble' as expected ── Error in `get_id(deimsid, "sites")`: deimsid 'fcc28bb3-551a-4396-819c-0589abc6be6f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_network_envcharacts(networkDEIMSID = TESTURLNetwork) test-get_network_envcharacts.R:25:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_network_parameters.R:25:3): Output of network parameters function constructs 'tibble' as expected ── Error in `get_id(deimsid, "sites")`: deimsid 'fcc28bb3-551a-4396-819c-0589abc6be6f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_network_parameters(networkDEIMSID = TESTURLNetwork) test-get_network_parameters.R:25:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_network_related_resources.R:25:3): Output of network related resources function constructs 'tibble' as expected ── Error in `get_id(deimsid, "sites")`: deimsid 'fcc28bb3-551a-4396-819c-0589abc6be6f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_network_related_resources(networkDEIMSID = TESTURLNetwork) test-get_network_related_resources.R:25:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") ── Error (test-get_network_research_topics.R:25:3): Output of network research topics function constructs ‘tibble’ as expected ── Error in `get_id(deimsid, "sites")`: deimsid 'fcc28bb3-551a-4396-819c-0589abc6be6f' cannot be used in test mode with resource = 'sites'. Backtrace: █ 1. └─ReLTER::get_network_research_topics(networkDEIMSID = TESTURLNetwork) test-get_network_research_topics.R:25:2 2. └─base::lapply(...) 3. └─ReLTER:::FUN(X[[i]], ...) 4. └─ReLTER:::get_id(deimsid, "sites") [ FAIL 7 | WARN 0 | SKIP 1 | PASS 273 ] Error: Test failures Execution halted R CMD check generated the following check_fail: 1. rcmdcheck_tests_pass #### Test coverage with [covr](https://covr.r-lib.org/) ERROR: Test Coverage Failed #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) No functions have cyclocomplexity >= 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found no issues with this package!


3. Other Checks

:heavy_check_mark: Package contains the following (potentially) obsolete packages:

See our Recommended Scaffolding for alternatives.


Package Versions

|package |version | |:--------|:---------| |pkgstats |0.0.3.59 | |pkgcheck |0.0.2.204 |


Editor-in-Chief Instructions:

Processing may not proceed until the items marked with :heavy_multiplication_x: have been resolved.

maurolepore commented 2 years ago

@oggioniale,

I'm sorry this is so frustrating. You've come a long way and I'm confident you'll get there soon.

You're running rcmdcheck::rcmdcheck(".") but I (and apparently the bot) is running devtools::test() and devtools::check(). You can also click on "Test Package" and "Check Package" on RStudio's "Build" tab:

Consider the project we've been using (https://rstudio.cloud/project/3335565) at this (last) commit:

commit 5f9b04796e446655761614b28cfecf693fa51526 (HEAD -> main, origin/main, origin/HEAD)
Author: Alessandro Oggioni <oggioniale@gmail.com>
  Date:   Wed Dec 22 15:14:42 2021 +0100

revert

Indeed I see that rcmdcheck::rcmdcheck(".") throws 0 errors, warnings, and notes. But devtools::test() shows the same 7 failures that the bot detects. They are all similar to this one:

Error (test-get_ilter_generalinfo.R:23:3): Output of ILTER general info function constructs 'tibble' as
          expected
Error in `get_id(deimsid, "sites")`: deimsid '5de9ae4c-c705-4cd5-b55a-2195c936f7fb' cannot be used in test mode with resource = 'sites'.
Backtrace:
 1. ReLTER::get_ilter_generalinfo(country_name = NA, site_name = NA) test-get_ilter_generalinfo.R:23:2
 2. base::lapply(...) /cloud/project/R/get_ilter_generalinfo.R:85:2
 3. ReLTER:::FUN(X[[i]], ...)
 4. ReLTER::get_id(deimsid, "sites") /cloud/project/R/get_site_info.R:48:2

Please run devtools::test() yourself to confirm.

oggioniale commented 2 years ago

@maurolepore, the last commits and PR solved the failures of test and check when execute the devtools::test() and devtools::check. As previously also `rcmdcheck::rcmdcheck(".")

Let me know please.

maurolepore commented 2 years ago

@ropensci-review-bot check package

ropensci-review-bot commented 2 years ago

Thanks, about to send the query.

ropensci-review-bot commented 2 years ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 2 years ago

Checks for ReLTER (v1.0.0)

git hash: c3383e93

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

Package License: GPL (>= 3)


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 36 files) and - 4 authors - 1 vignette - no internal data file - 28 imported packages - 33 exported functions (median 28 lines of code) - 35 non-exported functions in R (median 44 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 | 36| 92.1| | |files_vignettes | 1| 68.4| | |files_tests | 27| 97.6| | |loc_R | 1926| 83.5| | |loc_vignettes | 48| 8.2| | |loc_tests | 1355| 90.8| | |num_vignettes | 1| 64.8| | |n_fns_r | 68| 65.9| | |n_fns_r_exported | 33| 80.4| | |n_fns_r_not_exported | 35| 57.4| | |n_fns_per_file_r | 1| 0.2|TRUE | |num_params_per_fn | 1| 1.6|TRUE | |loc_per_fn_r | 38| 83.7| | |loc_per_fn_r_exp | 28| 60.2| | |loc_per_fn_r_not_exp | 44| 88.0| | |rel_whitespace_R | 1| 13.2| | |rel_whitespace_vignettes | 46| 15.1| | |rel_whitespace_tests | 13| 83.2| | |doclines_per_fn_exp | 24| 21.5| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 38| 60.5| | ---

1a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

#### 3a. Continuous Integration Badges [![R-CMD-check](https://github.com/oggioniale/ReLTER/workflows/R-CMD-check/badge.svg)](https://github.com/oggioniale/ReLTER/actions) **GitHub Workflow Results** |name |conclusion |sha |date | |:--------------------------|:----------|:------|:----------| |pages build and deployment |success |c3383e |2022-01-18 | |R-CMD-check |failure |33f6a8 |2022-01-18 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) R CMD check generated the following note: 1. checking dependencies in R code ... NOTE Namespaces in Imports field not imported from: ‘scales’ ‘stringr’ All declared Imports should be used. R CMD check generated the following check_fail: 1. rcmdcheck_imports_not_imported_from #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 61.28 The following files are not completely covered by tests: file | coverage --- | --- R/get_activity_info.R | 69.05% R/get_dataset_info.R | 64.2% R/get_id.R | 53.33% R/get_ilter_envcharacts.R | 55.56% R/get_ilter_parameters.R | 56.1% R/get_ilter_research_topics.R | 60.53% R/get_sos_procedurelist.R | 0% R/produce_site_map.R | 0% R/produce_site_qrcode.R | 60% R/taxon_id_pesi.R | 0% R/taxon_id_worms_refine.R | 0% R/taxon_id_worms.R | 0% #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) No functions have cyclocomplexity >= 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 1 potential issues: message | number of times --- | --- Lines should not be more than 80 characters. | 1


3. Other Checks

:heavy_check_mark: Package contains the following (potentially) obsolete packages:

See our Recommended Scaffolding for alternatives.


Package Versions

|package |version | |:--------|:---------| |pkgstats |0.0.3.86 | |pkgcheck |0.0.2.207 |


Editor-in-Chief Instructions:

Processing may not proceed until the items marked with :heavy_multiplication_x: have been resolved.

maurolepore commented 2 years ago

Yay! Congratulations @oggioniale! I'll start looking for reviewers.

In the mean time please try increase the test coverage. I see "Package coverage is 61.3% (should be at least 75%)".

Reading this short section may help: https://mastering-shiny.org/scaling-testing.html#code-coverage

maurolepore commented 2 years ago

@oggioniale,

Could you please list 3 people you think might be good reviewers for ReLTER and briefly explain why?

I would aim to use only one but your list of three -- along with your explanations -- would help me better understand the ways in which you think an external eye can add value.

oggioniale commented 2 years ago

@maurolepore

I would suggest:

  1. Will Bolton for his involvement in the activities of the project (eLTER-Plus) in which the ReLTER package was born.
  2. Daniel Nüst for the spatio-temporal and geoinformatics skills, but also for his focus on the principles of open and reproducible science.
  3. Carl Boettiger for his closeness to the activities of the community of LTER US (the ReLTER package was born in the European LTER community). He is a science adviser to NCEAS - National Center for Ecological Analysis and Synthesis, the ReLTER package is dedicated to the Ecological scientists community. But also for the interests in open science, data science, and ecoinformatics.
oggioniale commented 2 years ago

@maurolepore The last commit in the repo improve the code coverage (now more than 82%).

Ale

maurolepore commented 2 years ago

@ropensci-review-bot check package

ropensci-review-bot commented 2 years ago

Thanks, about to send the query.

ropensci-review-bot commented 2 years ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 2 years ago

Checks for ReLTER (v1.0.0)

git hash: 25650212

Package License: GPL (>= 3)


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 36 files) and - 4 authors - 1 vignette - no internal data file - 26 imported packages - 33 exported functions (median 28 lines of code) - 35 non-exported functions in R (median 44 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 | 36| 92.1| | |files_vignettes | 1| 68.4| | |files_tests | 32| 98.2| | |loc_R | 1974| 84.0| | |loc_vignettes | 48| 8.2| | |loc_tests | 1614| 92.2| | |num_vignettes | 1| 64.8| | |n_fns_r | 68| 65.9| | |n_fns_r_exported | 33| 80.4| | |n_fns_r_not_exported | 35| 57.4| | |n_fns_per_file_r | 1| 0.2|TRUE | |num_params_per_fn | 1| 1.6|TRUE | |loc_per_fn_r | 38| 83.7| | |loc_per_fn_r_exp | 28| 60.2| | |loc_per_fn_r_not_exp | 44| 88.0| | |rel_whitespace_R | 1| 13.2| | |rel_whitespace_vignettes | 46| 15.1| | |rel_whitespace_tests | 13| 85.5| | |doclines_per_fn_exp | 25| 23.5| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 40| 61.6| | ---

1a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

#### 3a. Continuous Integration Badges [![R-CMD-check](https://github.com/oggioniale/ReLTER/workflows/R-CMD-check/badge.svg)](https://github.com/oggioniale/ReLTER/actions) **GitHub Workflow Results** |name |conclusion |sha |date | |:--------------------------|:----------|:------|:----------| |pages build and deployment |success |256502 |2022-01-25 | |R-CMD-check |success |256502 |2022-01-25 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) rcmdcheck found no errors, warnings, or notes #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 82.27 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) No functions have cyclocomplexity >= 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found no issues with this package!


3. Other Checks

:heavy_check_mark: Package contains the following (potentially) obsolete packages:

See our Recommended Scaffolding for alternatives.


Package Versions

|package |version | |:--------|:---------| |pkgstats |0.0.3.88 | |pkgcheck |0.0.2.225 |


Editor-in-Chief Instructions:

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

maurolepore commented 2 years ago

@ropensci-review-bot assign @WillOnGit as reviewer

ropensci-review-bot commented 2 years ago

@WillOnGit added to the reviewers list. Review due date is 2022-02-18. Thanks @WillOnGit for accepting to review! Please refer to our reviewer guide.

ropensci-review-bot commented 2 years ago

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

maurolepore commented 2 years ago

@WillOnGit, although the bot suggests that the due date is 2022-02-18 you will have a bit longer than that as I am still looking for a second reviewer.

maurolepore commented 2 years ago

Dear @oggioniale FYI I'm still searching for reviewer 2. Sometimes it takes a while. Please be patient.

oggioniale commented 2 years ago

Dear @maurolepore do you have any news about the second reviewer?

maurolepore commented 2 years ago

Dear @oggioniale,

I'm sorry it's taking so long. I tried about 10 so far but unfortunately I haven't found one yet. It has happened before that I find two reviewers right away, and also that it takes quite some time.

To try move faster I'll ask for help or ideas for reviewers to other editors.

oggioniale commented 2 years ago

Dear @maurolepore

thank you very much!

maurolepore commented 2 years ago

@ropensci-review-bot assign @allisonhorst as reviewer

ropensci-review-bot commented 2 years ago

@allisonhorst added to the reviewers list. Review due date is 2022-03-10. Thanks @allisonhorst for accepting to review! Please refer to our reviewer guide.

ropensci-review-bot commented 2 years ago

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

maurolepore commented 2 years ago

@ropensci-review-bot set due date for @WillOnGit to 2022-03-10

ropensci-review-bot commented 2 years ago

Review due date for @WillOnGit is now 10-March-2022

maurolepore commented 2 years ago

@oggioniale, awesome news: The second reviewer will be @allisonhorst.

allisonhorst commented 2 years ago

Here is my review of the ReLTER package! Happy to answer any questions and provide clarification. Some of the testing issues may be on my end but I included anyway in case useful to see.

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

None.

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 6


Review Comments

Overall comments:

Checklist from ropensci-org/pkgreviewr package template

Session info

Test installation

Test local ReLTER install:

install(pkg_dir, dependencies = T, build_vignettes = T)

No issues.


Test install of ReLTER from GitHub with:

devtools::install_github("oggioniale/ReLTER", dependencies = T, build_vignettes = T)

No issues.


Check package integrity

Run checks on ReLTER source:

devtools::check() returns 0 errors, 0 warnings, 0 notes


Run tests on ReLTER source:

devtools::test(): I’m having real issues with this, with of warnings and errors thrown, terminated early, etc. (see below of the summary). I’d love to try this again & learn if I’m doing something wrong on my end, this doesn’t seem to align with the testing checks done by the editor or authors?

Maximum number of failures exceeded; quitting early.
You can increase this number by setting `options(testthat.progress.max_fails)` 

══ Results ═══════════════════════════════════════════════════════════════════════════════════════════════
Duration: 1138.7 s

[ FAIL 10 | WARN 8 | SKIP 0 | PASS 244 ]
══ Terminated early ═

Check ReLTER for goodpractice:

goodpractice::gp() returns: “It is good practice to

✖ fix this R CMD check ERROR: Package suggested but not available: ‘ISOcodes’ The suggested packages are required for a complete check. Checking can be attempted without them by setting the environment variable R_CHECK_FORCE_SUGGESTS to a false value. See section ‘The DESCRIPTION file’ in the ‘Writing R Extensions’ manual.”


Check package metadata files

Inspect

Spell check

devtools::spell_check(pkg_dir)

Check spelling:

README comments:

DESCRIPTION comments:

NAMESPACE comments:

None.


Check documentation

test ReLTER function help files:

help(package = "ReLTER") returns documentation as expected.


test ReLTER vignettes:

vignette(package = "ReLTER") returns single vignette with information from the README. Annotated / explanatory vignettes would be really helpful.


Test functionality:

The reference page descriptions of the function are the function name followed by “function,” but would be more helpful if they contained a brief description of what the function actually does instead (e.g. https://docs.ropensci.org/GSODR/reference/index.html)

I’ve noted specific recommendations for function use & documentation after the specific functions below.

Feedback on individual package functions

Comment throughout on function documentation: in many functions’ documentation there is a line “More information about DEIMS network ID in these pages: page, and page the complete list of ILTER networks.” I think the link text needs to be updated (e.g. to “DEIMS.iD information and Complete list of ILTER networks” instead of “page and page”).

get_site_info()

get_site_ODS()

get_network_envcharacts()

get_network_parameters()

get_network_research_topics()

get_network_related_resources()

get_network_sites()

get_ilter_envcharacts()

get_ilter_generalinfo()

get_ilter_parameters()

get_ilter_research_topics()

get_activity_info()

get_dataset_info()

produce_network_points_map()

produce_site_map()

produce_site_qrcode()

produce_site_parameters_pie()

produce_site_parameters_waffle()

get_sos_procedurelist()

taxon_id_pesi()

taxon_id_worms()


Inspect code:

Comments on package and code:

Review test suite:

Test coverage

covr::package_coverage(pkg_dir) returns:

══ Failed tests ════════════════════════════════════════════════════════════════
── Error (test-taxon_id_pesi.R:21:3): Output of taxon pesi function constructs 'tibble' as expected ──
Error: 
Backtrace:
    ▆
 1. └─ReLTER::taxon_id_pesi(table = table, taxaColumn = 3) at test-taxon_id_pesi.R:21:2
 2.   └─taxize::eubon_search(query = table[[taxaColumn]][i], providers = "pesi")
 3.     └─taxize:::eubon_error(res)

[ FAIL 1 | WARN 2 | SKIP 9 | PASS 234 ]
Error: Test failures
Execution halted

Inspect tests


End review.

I look forward to answering any questions you have about this feedback, and am happy to re-test/review as you work on revisions! -Allison

micha-silver commented 2 years ago

@allisonhorst Thank you for the excellent review. We're "on it".

oggioniale commented 2 years ago

@allisonhorst Many many thanks for your review. We working on the answers and on fixing the issues that you are open. In the next few days our answers and the commits for improve the quality of ReLTER package following your suggestions.

maurolepore commented 2 years ago

@oggioniale, great you're keen to respond quickly but please remember the review by @WillOnGit is still pending. If you do any work now please consider using a branch other than main. Working on main would force @WillOnGit to work against a moving target. In the end you'll need to consider both reviews jointly.

oggioniale commented 2 years ago

@maurolepore thanks you for suggestions. I was referring to put some answers to the comments of @allisonhorst as is done in reviews of scientific papers and, in the meantime, devote a specific branch to the issues.

oggioniale commented 2 years ago

Here is my review of the ReLTER package! Happy to answer any questions and provide clarification. Some of the testing issues may be on my end but I included anyway in case useful to see.

Dear @allisonhorst, thanks a lot for your comments, moreover, for your revisions, suggestions and issues. In line my answers.


Review Comments

Overall comments:

  • This seems like a valuable package to access and work with data for LTER users! It combines a lot of options for accessing information about networks, sites, and data that I imagine is very useful for LTER researchers working in R

This is exactly the main aim of ReLTER package: interact with the informations and data provided by eLTER tools. In addition, since the eLTER community is global and provides long-term ecological research data, in perspective will be useful for ecology researchers working in R.

  • A clearer statement of need & a bit more background about LTER / eLTER on the homepage & README could provide a nicer entryway for beginner users, or those who happen up on it. It dives into the details a bit quickly, and a broader intro & motivation would be helpful. It would also be great to have a section on other options for how users can interact with the LTER data.

We will do. As part of the eLTER community we take it for seems obvious that community practices are known, it is good to have an outside view. We should also describethe difference between LTER and eLTER.

  • I really like the welcoming CONTRIBUTING documentation ❤️

❤️

  • I think the package would really benefit from a few vignettes, beyond what is in the README, with examples including explanatory text & annotation describing what’s going on, how to find the DEIMS.iD being used, etc. that walks adopters through how and why they might use functions in the package. Also, weirdly, when I install from GitHub browseVignettes() returns “No vignettes found” – but following local install the one vignette does show up. Could be weirdness on my end.

In the few weeks, I started with the creation of "How to" pages describing some of the practical needs of researchers in the eLTER community (e.g. How to get the info of most than one LTER Sites?). Probably these are not true vignettes but can be a starting point.

  • It took me a while to figure out where to find the DEIMS.iD for sites, networks, activities, data, etc. (and to understand that these are different - e.g. from what I tried a DEIMS.iD for a network will fail on a function built for a site). If it is possible on the homepage to clearly link (e.g. in the README) to sites that list all networks, sites, activities, etc. that could be really helpful for newcomers to the package.

Another thing we took for seems obvious.

  • For a number of functions more information in the documentation would be helpful for new users. I’ve noted suggestions for specific functions below.

Thank you very much, we will consider all your suggestions to improve the use of the package.


Inspect code:

Comments on package and code:

  • Package function and argument naming conventions are intuitive and consistent. I note that the authors follow a verb-object naming scheme (instead of object-verb as recommended by ROpenSci package guidelines).
  • Code is readable and organized throughout, and appears to follow the tidyverse style guide
  • Overall, I think the package mostly adheres to ROpenSci Packaging Guide. Minor note: Section 1.16 of the ROpenSci Packaging Guide recommends “Avoid starting the description with the package name or”This package …”
  • I don’t note any code duplication (but also don’t have a lot of experience looking for it, so might be missing something)
  • Re: dependencies, installing this package seems like it may require a lot of updates for users (given that all dependencies except terra specify a minimum version). This may be necessary, else consider from the ROpenSci Packaging Guide (section 1.13): “If you know for a fact that your package will break below a certain dependency version, specify it explicitly. But if you don’t, then no need to specify a minimum dependency.” I’ll add a massive caveat here that I know basically nothing about handling dependencies, so you might just ignore this comment :)

End review.

I look forward to answering any questions you have about this feedback, and am happy to re-test/review as you work on revisions! -Allison

We hope not to give you too much work yet, we will strive to improve the package in the way you suggested. Alessandro

maurolepore commented 2 years ago

Dear @WillOnGit, please :pray: note the due date is only a few days ahead: 2022-03-10

WillOnGit commented 2 years ago

Hi all, sorry but I've been mad busy lately. I'll begin my review this afternoon and will submit tomorrow evening at the latest.

WillOnGit commented 2 years ago

I've run into a lot of technical problems while writing the review, and while I could submit something today I think it's best to revert to the original due date of tomorrow for the submission. The issues are all solved now so barring any other complications there should be no trouble submitting on time still.

I'll elaborate on these problems in the review itself but basically there's a good chance they have nothing to do with the package itself.

Sorry for the additional delay!

WillOnGit commented 2 years ago

OK, here's my review. I haven't commented on some of the more detailed aspects of the review (such as strict adherence to the various reviewing/packaging guides) but please let me know if I've omitted anything crucial and I'll update it.

I'll keep an eye out for any follow up questions that may arise but will be busy tomorrow afternoon.

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

I work on the eLTER project separately from the authors. I attended a workshop on the ReLTER package given by Micha Silver. I've also corresponded with Alessandro and Paolo about a separate R package.

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 8


Review Comments

Overall comments

On the whole, I really liked the package - once I could get the damn thing installed!

The package would be good enough if it just standardised the busywork of fetching stuff from APIs such as DEIMS, but it provides lots of extra bonuses (like produce_site_qrcode, produce_network_points_map and taxon_id_worms) that go above and beyond the basics. The good thing is, if you don't like the output of these bonuses you can do your own thing with the data the package gives you.

The documentation and other supporting material is very good, and I'm a stickler for that kind of thing. This helps elevate the package from being functional to enjoyable to use.

I will definitely use this package myself from now on, which is really the ultimate endorsement!

Technical info

> sessionInfo()
R version 4.0.3 (2020-10-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 17763)

Matrix products: default

locale:
[1] LC_COLLATE=English_United Kingdom.1252 
[2] LC_CTYPE=English_United Kingdom.1252   
[3] LC_MONETARY=English_United Kingdom.1252
[4] LC_NUMERIC=C                           
[5] LC_TIME=English_United Kingdom.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods  
[7] base     

other attached packages:
[1] ReLTER_1.0.0

loaded via a namespace (and not attached):
...

Installation

I only tested the devtools::install_github() installation as that is all that is indicated in the documentation.

Installation was extremely difficult and frustrating (~4 hours); an important caveat is that circumstances meant I only had two environments to use, Windows and Linux, both of which had limited functionality in different ways. In the end, I got the package working on a corporate Windows laptop after battling antivirus software and using several additional install.packages invocations. I would strongly recommend further testing on "normal", modern Windows and Linux environments to confirm that my experience was a product of the unusual setup I had to work with.

Because of similar issues, I could not use the ropensci-org/pkgreviewr template.

Installation comments

The amount of dependencies is very large, which is understandable considering some of the advanced functionalities provided, especially (I assume) the graphical/GIS features. This meant that installation was slow, which may be offputting for users wanting to try the package out. Perhaps a minimal version/install option of the package would be nice for low resource users or those mainly dealing with DEIMS site metadata.

This was especially noticeable in my case where the installation failed, and on each failure I had to wait a long time before seeing whether it was working again or not.

Testing

No problems here. All tests seem reasonable and all functions are covered. Everything passed locally, results are below.

> devtools::test()
i Loading ReLTER

ReLTER is specially drafted for the LTER community.

...

== Results ===========================================================================
Duration: 567.1 s

[ FAIL 0 | WARN 2 | SKIP 0 | PASS 355 ]

Code style

The code looks well-written on the whole. The only suggestion I have is to introduce more "internal" functions (like get_site_general from get_site_info) to standardise procedures and cut out some boilerplate code.

As an example, in get_network_sites there are a few lines at the beginning setting up a URL, defining retries, standardising DEIMS IDs, etc. Similar things appear in produce_network_points_map. Some sort of wrapper(s) for contacting the DEIMS API looks like it would be good to replace all these things with something like getFromDeims("foo"). As an example (a vain one, at that - run git blame on it and you'll see what I mean...), from a similar piece of work see the "normaliseDeimsID" function here: https://github.com/eLTER-RI/deimsPy/blob/main/deims.py.

Documentation

In terms of the Divio documentation system (my favourite way of documenting things), the reference material is the strongest, the explanation and tutorials are good (plenty of examples), and the how-to guides are lacking, most likely in the format of a few vignettes to expand on the README examples. Some other points are:

Functionality

I've only included functions where I had specific comments here, so anything not mentioned you can assume I:

Some functions didn't do what I expected from the name (e.g. produce_site_map: I assumed you would just pass it a DEIMS ID and it would make a map of the site and its immediate surroundings) but on the whole were well-named and intuitive—once you're familiar with the terminology, although this package will largely be used by folks familiar with eLTER anyway I imagine.

oggioniale commented 2 years ago

OK, here's my review. I haven't commented on some of the more detailed aspects of the review (such as strict adherence to the various reviewing/packaging guides) but please let me know if I've omitted anything crucial and I'll update it.

I'll keep an eye out for any follow up questions that may arise but will be busy tomorrow afternoon.

...

Dear @WillOnGit, thanks you for your review. Your comments, suggestions and improvements request, based on your programming experience, are a powerful impetus for improving the package. In the next days our commits for answer to your issues.

Ale

maurolepore commented 2 years ago

Thanks @WillOnGit for your review!

@oggioniale, go ahead and incorporate the great feedback. Let me know if you need me.

In general aim for 3 weeks for review, 2 weeks for subsequent changes, and 1 week for reviewer approval of changes. -- https://devdevguide.netlify.app/editorguide.html#during-review

I'll follow below with two admin comments.

maurolepore commented 2 years ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/485#issuecomment-1049290009 time 6

ropensci-review-bot commented 2 years ago

Logged review for allisonhorst (hours: 6)