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

emodnet.wfs: Access EMODnet Web Feature Service data through R #653

Open
13 of 29 tasks
maelle opened this issue Sep 3, 2024 · 62 comments
Open
13 of 29 tasks

emodnet.wfs: Access EMODnet Web Feature Service data through R #653

maelle opened this issue Sep 3, 2024 · 62 comments

Comments

@maelle
Copy link
Member

maelle commented Sep 3, 2024

Submitting Author Name: Maëlle Salmon
Submitting Author Github Handle: @maelle
Other Package Authors Github handles: (comma separated, delete if none) @salvafern, @annakrystalli
Repository: https://github.com/EMODnet/emodnet.wfs/
Version submitted: 2.0.2
Submission type: Standard
Editor: @MargaretSiple-NOAA
Reviewers: @robitalec, @LizHareDogs

Archive: TBD
Version accepted: TBD
Language: en


  • Paste the full DESCRIPTION file inside a code block below:
Package: emodnet.wfs
Title: Access EMODnet Web Feature Service data through R
Version: 2.0.2
Authors@R: c(
    person("Anna", "Krystalli", , "[email protected]", role = "aut",
           comment = c(ORCID = "0000-0002-2378-4915")),
    person("Salvador", "Fernández-Bejarano", , "[email protected]", role = c("aut", "cre"),
           comment = c(ORCID = "0000-0003-0535-7677")),
    person("Thomas J", "Webb", , "[email protected]", role = "ctb"),
    person("European Marine Observation Data Network (EMODnet) Biology project", "European Commission's Directorate - General for Maritime Affairs and Fisheries (DG MARE)", , "[email protected]", role = "cph"),
    person("VLIZ (VLAAMS INSTITUUT VOOR DE ZEE)", , , "[email protected]", role = "fnd"),
    person("Maëlle", "Salmon", , "[email protected]", role = "aut",
           comment = c(ORCID = "0000-0002-2815-0399"))
  )
Description: Access and interrogate EMODnet (European Marine Observation and Data Network) 
    Web Feature Service data through R.
License: MIT + file LICENSE
URL: https://emodnet.github.io/emodnet.wfs/,
    https://github.com/EMODnet/emodnet.wfs
BugReports: https://github.com/EMODnet/emodnet.wfs/issues
Depends: 
    R (>= 3.6.0)
Imports: 
    checkmate,
    cli,
    dplyr,
    lifecycle,
    magrittr,
    memoise,
    ows4R (>= 0.4),
    purrr,
    rlang,
    sf,
    tibble,
    utils,
    whoami
Suggests: 
    covr,
    httptest,
    knitr,
    mapview,
    readr,
    rmarkdown,
    skimr,
    testthat (>= 3.1.2),
    testthis,
    withr
Config/Needs/readme: rerddap
VignetteBuilder: 
    knitr
Config/testthat/edition: 3
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.2
SystemRequirements: C++11, GDAL (>= 2.0.1), GEOS (>= 3.4.0), PROJ (>=
    4.8.0)
Remotes: 
    eblondel/ows4R

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
    • data validation and testing
    • 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):

The package allow interrogation of and access to EMODnet’s, European Marine Observation and Data Network, geographic vector data.

  • Who is the target audience and what are scientific applications of this package?

The target audience of the package are EMODnet users that might need programmatic access to EMODnet's geographic vector data.
The package allows to include EMODnet vector data into scientific pipelines without needing to manually explore and download data from the EMODnet Geographic Viewer.
The data covers seven disciplinary themes (bathymetry, geology, physics, chemistry, biology, seabed habitats and human activities).

No, to our knowledge emodnet.wfs is the only package that provides access to EMODnet data in R though the EMODnet Web Feature
Services.
The emodnet.wfs package was developed in collaboration with other EMODnet members.

There are in total three ways to access EMODnet data that complement each other and which we documented in emodnet.wfs README:

N/A

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

  • Explain reasons for any pkgcheck items which your package is unable to pass.

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 emodnet.wfs (v2.0.2.9000)

git hash: 5b516d4c

  • ✔️ Package name is available
  • ✔️ 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 93.3%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.

Package License: MIT + file LICENSE


1. Package Dependencies

Details of Package Dependency Usage (click to open)

The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.

type package ncalls
internal emodnet.wfs 36
internal base 26
internal graphics 4
internal stats 1
imports magrittr 14
imports purrr 13
imports checkmate 3
imports memoise 3
imports dplyr 1
imports sf 1
imports tibble 1
imports utils 1
imports whoami 1
imports cli NA
imports lifecycle NA
imports ows4R NA
imports rlang NA
suggests covr NA
suggests httptest NA
suggests knitr NA
suggests mapview NA
suggests readr NA
suggests rmarkdown NA
suggests skimr NA
suggests testthat NA
suggests testthis NA
suggests withr NA
linking_to NA NA

Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.

emodnet.wfs

namespace_layer_names (4), emodnetwfs_user_agent (3), get_abstract_null (2), get_layer_default_crs (2), get_layer_format (2), get_layer_metadata (2), get_service_name (2), guess_layer_format (2), check_layer_crs (1), check_service_name (1), check_wfs (1), checkmate_crs (1), cli_alert_danger (1), cli_alert_info (1), cli_alert_success (1), deprecate_msg_service_version (1), emodnet_get_all_wfs_info (1), emodnet_get_layers (1), emodnet_init_wfs_client (1), emodnetwfs_collaborators (1), ews_get_layer (1), get_layer_bbox (1), get_service_url (1), layer_attribute_descriptions (1), layer_attributes_get_names (1)

base

c (7), attributes (2), capabilities (2), format (2), parent.frame (2), strsplit (2), as.character (1), length (1), paste (1), pretty (1), regmatches (1), sum (1), suppressWarnings (1), try (1), version (1)

magrittr

%>% (14)

purrr

map_chr (11), map (1), map2 (1)

graphics

text (2), title (2)

checkmate

assert_character (3)

memoise

memoise (3)

dplyr

mutate (1)

sf

st_crs (1)

stats

df (1)

tibble

tibble (1)

utils

packageVersion (1)

whoami

gh_username (1)

NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately.


2. 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 10 files) and
  • 3 authors
  • 3 vignettes
  • no internal data file
  • 13 imported packages
  • 11 exported functions (median 6 lines of code)
  • 62 non-exported functions in R (median 6 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

All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown() function

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 10 56.5
files_vignettes 3 89.3
files_tests 162 99.9
loc_R 497 45.5
loc_vignettes 409 70.5
loc_tests 39409 99.8 TRUE
num_vignettes 3 91.5
n_fns_r 73 66.4
n_fns_r_exported 11 48.6
n_fns_r_not_exported 62 71.3
n_fns_per_file_r 4 65.4
num_params_per_fn 4 51.1
loc_per_fn_r 6 13.1
loc_per_fn_r_exp 6 11.7
loc_per_fn_r_not_exp 6 15.0
rel_whitespace_R 19 48.2
rel_whitespace_vignettes 56 85.3
rel_whitespace_tests 0 54.5
doclines_per_fn_exp 32 36.5
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 58 68.0

2a. Network visualisation

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


3. goodpractice and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

R-CMD-check.yaml

GitHub Workflow Results

id name conclusion sha run_number date
10668665094 Commands skipped 77ed52 314 2024-09-02
10668583391 lint-changed-files success 643dd5 13 2024-09-02
10678310116 pages build and deployment success 7b12c0 56 2024-09-03
10678239121 pkgcheck success 5b516d 123 2024-09-03
10678239120 pkgdown success 5b516d 319 2024-09-03
10678239122 R-CMD-check success 5b516d 1210 2024-09-03
10678239118 test-coverage.yaml success 5b516d 293 2024-09-03
10674669352 Test-Services success 77ed52 533 2024-09-03
10674607777 test-without-fixtures success 77ed52 387 2024-09-03

3b. goodpractice results

R CMD check with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 93.31

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found no issues with this package!


Package Versions

package version
pkgstats 0.1.6.17
pkgcheck 0.1.2.58


Editor-in-Chief Instructions:

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

@adamhsparks
Copy link
Member

adamhsparks commented Sep 4, 2024

Hi @maelle, I've got a NOTE when I run checks on your package with the documentation.

checking Rd files ... [0s/0s] NOTE
   checkRd: (-3) emodnet_get_layers.Rd:40-41: Lost braces
       40 | \item{character string or character vector of length 1.
          |      ^
   checkRd: (-3) emodnet_get_layers.Rd:42-44: Lost braces
       42 | \item{character vector of length equal to the length of layers.
          |      ^
   checkRd: (-3) emodnet_get_layers.Rd:45-48: Lost braces
       45 | \item{named character vector. Each filter will be applied to the layer

Also, the tests seem to take quite a while to run. Locally I edited the DESCRIPTION file to include Config/testthat/parallel: true so that it was a bit faster.

Could you check on the NOTE and see if you can fix it and I'll assign an editor in the meantime?

@adamhsparks
Copy link
Member

@ropensci-review-bot assign @jhollist as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @jhollist is now the editor

@maelle
Copy link
Member Author

maelle commented Sep 4, 2024

Thanks @adamhsparks!

What exactly do you mean by running checks on the package with the documentation? I changed the markup for those items but would like to re-run the checks as you did to ensure I did fix the problem.

I have added that field to DESCRIPTION, so the tests run faster now, thank you!

@adamhsparks
Copy link
Member

adamhsparks commented Sep 4, 2024

@maelle, sorry, that was worded poorly or missing a comma. It was just devtools::check(). The most recent commits appear to have fixed the NOTE with the "{" in checks.

@adamhsparks
Copy link
Member

@ropensci-review-bot assign @MargaretSiple-NOAA as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @MargaretSiple-NOAA is now the editor

@MargaretSiple-NOAA
Copy link

@ropensci-review-bot seeking reviewers

@ropensci-review-bot
Copy link
Collaborator

Please add this badge to the README of your package repository:

[![Status at rOpenSci Software Peer Review](https://badges.ropensci.org/653_status.svg)](https://github.com/ropensci/software-review/issues/653)

Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news

@MargaretSiple-NOAA
Copy link

Editor checks:

  • Documentation: The package has sufficient documentation available online (README, pkgdown docs) to allow for an assessment of functionality and scope without installing the package. In particular,
    • Is the case for the package well made?
    • [X ] Is the reference index page clear (grouped by topic if necessary)?
    • Are vignettes readable, sufficiently detailed and not just perfunctory?
  • Fit: The package meets criteria for fit and overlap.
  • Installation instructions: Are installation instructions clear enough for human users?
  • Tests: If the package has some interactivity / HTTP / plot production etc. are the tests using state-of-the-art tooling?
  • Contributing information: Is the documentation for contribution clear enough e.g. tokens for tests, playgrounds?
  • License: The package has a CRAN or OSI accepted license.
  • Project management: Are the issue and PR trackers in a good shape, e.g. are there outstanding bugs, is it clear when feature requests are meant to be tackled?

Editor comments

Great documentation and package setup. Thank you for being thorough!

A couple very minor notes:

  • I suggest changing the name of the intro vignette "emodnet.wfs Case Study: Accessing and mapping EMODnet data" to just "Accessing and mapping EMODnet data" and moving to top of article list.
  • I'm not totally sure what tokens / playgrounds would look like for this package but I found good contribution guidelines on the repo and IMO those are more than sufficient for contribution information.
  • It looks like there are just a couple bugs in the Issues but they are actively being worked on.

@MargaretSiple-NOAA
Copy link

@ropensci-review-bot add @robitalec to reviewers

@ropensci-review-bot
Copy link
Collaborator

@robitalec added to the reviewers list. Review due date is 2024-10-14. Thanks @robitalec for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

@ropensci-review-bot
Copy link
Collaborator

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

@MargaretSiple-NOAA
Copy link

@ropensci-review-bot add @LizHareDogs to reviewers

@ropensci-review-bot
Copy link
Collaborator

@LizHareDogs added to the reviewers list. Review due date is 2024-10-17. Thanks @LizHareDogs for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

@ropensci-review-bot
Copy link
Collaborator

Checks for emodnet.wfs (v2.0.2.9000)

git hash: d98df335

  • ✔️ Package name is available
  • ✔️ 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 93.3%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.

Package License: MIT + file LICENSE


1. Package Dependencies

Details of Package Dependency Usage (click to open)

The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.

type package ncalls
internal emodnet.wfs 36
internal base 26
internal graphics 4
internal stats 1
imports magrittr 14
imports purrr 13
imports checkmate 3
imports memoise 3
imports dplyr 1
imports sf 1
imports tibble 1
imports utils 1
imports whoami 1
imports cli NA
imports lifecycle NA
imports ows4R NA
imports rlang NA
suggests covr NA
suggests httptest NA
suggests knitr NA
suggests mapview NA
suggests readr NA
suggests rmarkdown NA
suggests skimr NA
suggests testthat NA
suggests testthis NA
suggests withr NA
linking_to NA NA

Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.

emodnet.wfs

namespace_layer_names (4), emodnetwfs_user_agent (3), get_abstract_null (2), get_layer_default_crs (2), get_layer_format (2), get_layer_metadata (2), get_service_name (2), guess_layer_format (2), check_layer_crs (1), check_service_name (1), check_wfs (1), checkmate_crs (1), cli_alert_danger (1), cli_alert_info (1), cli_alert_success (1), deprecate_msg_service_version (1), emodnet_get_all_wfs_info (1), emodnet_get_layers (1), emodnet_init_wfs_client (1), emodnetwfs_collaborators (1), ews_get_layer (1), get_layer_bbox (1), get_service_url (1), layer_attribute_descriptions (1), layer_attributes_get_names (1)

base

c (7), attributes (2), capabilities (2), format (2), parent.frame (2), strsplit (2), as.character (1), length (1), paste (1), pretty (1), regmatches (1), sum (1), suppressWarnings (1), try (1), version (1)

magrittr

%>% (14)

purrr

map_chr (11), map (1), map2 (1)

graphics

text (2), title (2)

checkmate

assert_character (3)

memoise

memoise (3)

dplyr

mutate (1)

sf

st_crs (1)

stats

df (1)

tibble

tibble (1)

utils

packageVersion (1)

whoami

gh_username (1)

NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately.


2. 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 10 files) and
  • 3 authors
  • 3 vignettes
  • no internal data file
  • 13 imported packages
  • 11 exported functions (median 6 lines of code)
  • 62 non-exported functions in R (median 6 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

All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown() function

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 10 56.4
files_vignettes 3 89.3
files_tests 162 99.9
loc_R 497 45.5
loc_vignettes 405 70.1
loc_tests 39409 99.9 TRUE
num_vignettes 3 91.5
n_fns_r 73 66.5
n_fns_r_exported 11 48.6
n_fns_r_not_exported 62 71.4
n_fns_per_file_r 4 65.5
num_params_per_fn 4 51.2
loc_per_fn_r 6 13.1
loc_per_fn_r_exp 6 11.6
loc_per_fn_r_not_exp 6 14.9
rel_whitespace_R 19 48.1
rel_whitespace_vignettes 56 85.2
rel_whitespace_tests 0 54.5
doclines_per_fn_exp 32 36.6
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 58 68.1

2a. Network visualisation

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


3. goodpractice and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

R-CMD-check.yaml

GitHub Workflow Results

id name conclusion sha run_number date
11227084499 R-CMD-check success d98df3 1220 2024-10-08
11319160507 Test-Services success d98df3 574 2024-10-14
11319086114 test-without-fixtures success d98df3 428 2024-10-14

3b. goodpractice results

R CMD check with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 93.31

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found no issues with this package!


Package Versions

package version
pkgstats 0.1.6.19
pkgcheck 0.1.2.61


Editor-in-Chief Instructions:

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

@robitalec
Copy link
Member

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

  • 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 have been confirmed.
  • Performance: Any performance claims of the software have 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: 4.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

Thanks to the authors of emodnet.wfs for submitting this well presented package. It was a pleasure to explore the available data from this source that I was not previously familiar with.

emodnet.wfs checked lots of boxes in terms of packaging guide and what I would expect as a new user including:

  • a good overview of functions in the README and introductory vignette
  • sf package used instead of older spatial packages
  • good test coverage

On my local machine, R CMD check returned 0 errors/warnings/notes and goodpractice checks looked good.

My four main comments are related to: 1) function naming, 2) function documentation, 3) introductory vignette, and 4) returned objects.

Function naming

Given the purpose of the emodenet.wfs package is to provide access to emodnet data through R, it is understandable and expected that the names are inherited from the broader EMODnet documentation. Looking at the NAMESPACE, I see:

  • emodnet_*
  • wfs_*
  • layer_*
  • attribute(s)_*

and a variety of operators/modifiers including “get_”, “get_all”, “_info”, “client”, “descriptions”, “get_names”, “summarise”, “tbl”.

My suggestion is to consider renaming functions to provide a more intuitive scheme for users. I understand this requires deprecating old functions, future breaking changes, etc - generally a fair amount of work. But I think it's important that function names are intuitive for users and follow a coherent scheme. My suggestions are to first remove all instances of “emodnet” since that is implied by the package name. Next, focuse on maybe three families of functions: “wfs_”, “layer_”, “attribute_”. Then reduce the length of function names and clarify the operation or expected output by including one or two specific operators. For instance, just by reading the function names, what might a user expect the differences are between “emodnet_wfs” and “emodnet_init_wfs_client”? Or for example, is there a risk that layer_attribute_* and layer_attributes_* provides a frustrating user experience if they miss this difference in pluralization?

My related suggestion is to consider including a dictionary of this scheme somewhere in the README or introductory vignette. I am not sure exactly where/if this already exists in the broader EMODnet documentation but I think a description of each family of functions wfs/layer/attributes would be useful for new users getting started with the package.

Function documentation

The documentation for this package is not extensively detailed. For example, the description sections for R functions are missing except for 1 function (emodnet_get_layers). I would suggest including a paragraph to describe each exported function so the user sees more than the title and list of parameters when they first open the help pages. I would also suggest including the name of the function in the parameters when an argument for a function is the output from another function, eg. in R/info.R: @param wfs …, from emodnet_init_wfs_client.

Introductory vignette

I appreciated the suggestion from @MargaretSiple-NOAA but I think one step further for renaming the introductory vignette is to highlight that it is the introductory vignette with something like “getting started” or “introduction to” etc. At the moment, the introductory vignette on the pkgdown site is nestled between the other two vignettes making it unclear which vignette a user should start with.

The word “lot” is used a couple of times but it is unclear what this word refers to with respect to emodnet.wfs functions (wfs/layers/attributes).

Consider using {knitr} tables or similar functions to tidy up the printing of outputs.

This chunk feels like a pretty non-introductory chunk of code. Not sure if it's required or could be simplified.

There is an error related to mapview/webshot in the pkgdown/rendered vignette:

# Transform to Polygon geometry type from Multisurface
if (unique(st_geometry_type(habitats_directive_layers)) == "MULTISURFACE") {
  habitats_directive_layers <- habitats_directive_layers %>%
    st_cast(to = "GEOMETRYCOLLECTION") %>%
    st_collection_extract(type = "POLYGON")
}

# Visualize
map <- mapview(habitats_directive_layers, zcol = "habitat_description", burst = TRUE)

map
#> Error in loadNamespace(name): there is no package called 'webshot'

Returned objects

The argument "reduce_layers" could use more explanation. The documentation suggests reducing layers will be attempted, but what is an example of reasons which reducing might fail?

Why does a single layer return a list? I think it's intuitive to match the names of multiple layers requested to a named list but wouldn't a user specifying a single layer expect an sf object to be directly returned?

> result <- emodnet_get_layers(
+     service = "biology",
+     layers = "mediseh_zostera_m_pnt"
+ )
✔ WFS client created successfullyService: "https://geo.vliz.be/geoserver/Emodnetbio/wfs"Version: "2.0.0"
> class(result)
[1] "list"

Minor comments

Given the overlap in authorship, if the authors are aware of how EMODnetWCS differs from emodnet.wfs or what the roadmap looks like for EMODnetWCS, it might be useful to include it in the README under other web services. Are these also additional tools to reference there? Could you add more precision to how "these three ways to access EMODnet complement each other"?

Codecov badge present in README twice, pointing to the root and tree/main, giving different code coverage percentages.

The authors could use cffr::cff_write() to generate a citation file.

Are there any rate limits/rules for the API? If so, could these be mentioned in the README and vignettes?

The skimr package is listed in the Suggests only to be used once in the ECQL filtering vignette. Is it a necessary dependency? I find reading through the vignette that the single skimr line returns an unexpected/different/confusing output to the previous outputs.

Some broken links detected with {urlchecker}.

> urlchecker::url_check()
! Warning: vignettes/emodnet.wfs.Rmd:199:100 Moved
Blondel, Emmanuel. (2020, May 27). ows4R: R Interface to OGC Web-Services (Version 0.1-5). Zenodo. http://doi.org/10.5281/zenodo.3860330
                                                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                                                                   https://doi.org/10.5281/zenodo.3860330Error: README.md:67:70 400: Bad Request
| geology_sea_floor_bedrock                                       | <https://drive.emodnet-geology.eu/geoserver/bgr/wfs>                          |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:69:70 400: Bad Request
| geology_submerged_landscapes                                    | <https://drive.emodnet-geology.eu/geoserver/bgs/wfs>                          |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:66:70 400: Bad Request
| geology_marine_minerals                                         | <https://drive.emodnet-geology.eu/geoserver/gsi/wfs>                          |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:68:70 400: Bad Request
| geology_seabed_substrate_maps                                   | <https://drive.emodnet-geology.eu/geoserver/gtk/wfs>                          |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:65:70 400: Bad Request
| geology_events_and_probabilities                                | <https://drive.emodnet-geology.eu/geoserver/ispra/wfs>                        |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:64:70 400: Bad Request
| geology_coastal_behavior                                        | <https://drive.emodnet-geology.eu/geoserver/tno/wfs>                          |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
! Warning: README.md:26:11 Moved
Services](https://emodnet.ec.europa.eu/en/data). [Web Feature services
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          https://emodnet.ec.europa.eu/en/data-0Error: README.md:59:70 400: Bad Request
| biology_occurrence_data                                         | <https://geo.vliz.be/geoserver/Dataportal/wfs>                                |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:58:70 400: Bad Request
| biology                                                         | <https://geo.vliz.be/geoserver/Emodnetbio/wfs>                                |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:90:16 400: Bad Request
#> ℹ Service: "https://geo.vliz.be/geoserver/Emodnetbio/wfs"
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:95:17 400: Bad Request
#> ....|-- url: https://geo.vliz.be/geoserver/Emodnetbio/wfs
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:107:16 400: Bad Request
#> ℹ Service: "https://geo.vliz.be/geoserver/Emodnetbio/wfs"
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:271:16 400: Bad Request
#> ℹ Service: "https://geo.vliz.be/geoserver/Emodnetbio/wfs"
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:339:16 400: Bad Request
#> ℹ Service: "https://geo.vliz.be/geoserver/Emodnetbio/wfs"
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:62:70 400: Bad Request
| chemistry_contaminants                                          | <https://geoserver.hcmr.gr/geoserver/EMODNET_SHARED/wfs>                      |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:57:70 400: Bad Request
| bathymetry                                                      | <https://ows.emodnet-bathymetry.eu/wfs>                                       |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:70:70 400: Bad Request
| human_activities                                                | <https://ows.emodnet-humanactivities.eu/wfs>                                  |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:73:70 400: Bad Request
| seabed_habitats_individual_habitat_map_and_model_datasets       | <https://ows.emodnet-seabedhabitats.eu/geoserver/emodnet_open_maplibrary/wfs> |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: README.md:72:70 400: Bad Request
| seabed_habitats_general_datasets_and_products                   | <https://ows.emodnet-seabedhabitats.eu/geoserver/emodnet_open/wfs>            |
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Error: vignettes/emodnet.wfs.Rmd:29:338 404: Not Found
For this tutorial we will make use of the `sf`,  `dplyr`  and `mapview` packages. The simple features `sf` package is a well known standard for dealing with geospatial vector data. The package `dplyr` is a strong library for data manipulation. This package also loads `magrittr`'s pipe operator `%>%` (you could also use the [base pipe](https://r4ds.hadley.nz/workflow-pipes.html)), which allows to write pipelines in R. To visualize geometries,  `mapview` will create quick interactive maps.
                                                                                                                                                                                                                                                                                                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
! Warning: vignettes/emodnet.wfs.Rmd:40:97 Moved
With the emodnet.wfs package, we can explore and combine the data served by the [EMODnet lots ](https://www.emodnet.eu/en/portals) through [OGC Web Feature Services](https://en.wikipedia.org/wiki/Web_Feature_Service) or WFS.
                                                                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                                                                https://emodnet.ec.europa.eu/en/emodnet-themes

@maelle
Copy link
Member Author

maelle commented Oct 16, 2024

Thank you for this insightful review @robitalec!! 🙏

@MargaretSiple-NOAA
Copy link

Thank you @robitalec !

@LizHareDogs
Copy link

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

  • Briefly describe any working relationship you have (had) with the package authors.
  • [X ] 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:

  • [X ] A statement of need: clearly stating problems the software is designed to solve and its target audience in README
  • [X ] Installation instructions: for the development version of package and any non-standard dependencies in README
  • [X ] Vignette(s): demonstrating major functionality that runs successfully locally
  • [*** ] Function Documentation: for all exported functions
  • [X ] 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).
    Partially: the community guidelines and contribution guidelines are not available.

Functionality

  • [X ] Installation: Installation succeeds as documented.
  • [X ] Functionality: Any functional claims of the software have been confirmed.
  • [NA ] Performance: Any performance claims of the software have been confirmed.
  • [X ] 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.
  • [X ] Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 25

  • [X ] 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

I am reviewing as part of the Champions program. My mentor is François Michonneau (@fmichonneau) and I have credited him with some of the suggestions below.

Line numbers mentioned below are in the unrendered files.

README.md

Readthrough

From the perspective of someone who has not worked with this kind of data, it would be helpful if the introduction (which starts at line 21) could provide a little more context. What is geographic
vector data? I'm new to this kind of data so it would be helpful to know in a less technical way what it is.

Line 26
"though" should be "through"

line 78
Not sure if I can use "View"

line 184
What does CRS stand for (it's mentioned a few times later)?

Line 313
what is an sf object? What is the difference between this and how the data were retrieved before?

Following instructions on Mac OS using Emacs, Emacs SPeaks Statistics

Line 78
I am not able to evaluate the View command on my system. This is not a problem with the package.

Otherwise, sample commands work as expected.

Vignette

All the commands in the vignette work for me.

Line 24 has the heading "data product." The meaning of this isn't clear to me. Will this be instructions on how to access a data product or how to create a data product?

Line 109 has this command:
map <- mapview(habitats_directive_layers, zcol = "habitat_description", burst = TRUE)

I am able to run this command, which opens a browser. I can't check the map with the screen reader, but the text on the screen seems appropriate.

In the rendered vignette at
the following error is displayed for the mapview command:

#> Error in loadNamespace(name): there is no package called 'webshot'

devtools

No problems with:

  • check()
  • test()
  • run_examples()

check() and test() find no problems.

pkgcheck

I am unable to do this locally on my Mac. Here's the error:

Error in normalizePath(path, mustWork = TRUE) : 
  path[1]="/var/folders/74/jdv3ddd512l0stdy3jzk45b80000gn/T//RtmpjIjxfG/filed972ede8640.txt": No such file or directory
> traceback()
6: normalizePath(path, mustWork = TRUE)
5: brio::read_lines(f_out)
4: pkgstats::ctags_test()
3: withCallingHandlers(expr, message = function(c) if (inherits(c, 
       classes)) tryInvokeRestart("muffleMessage"))
2: suppressMessages(pkgstats::ctags_test())
1: pkgcheck()
>

This is apparently known because the normalizePath documentation says

The canonical form of paths may not be what you expect. For example, on macOS absolute paths such as ‘/tmp’ and ‘/var’ are symbolic links

ropensci-review-bot

Everything here checks ou, including pkgcheck, which ccouldn't be run locally.

Coverage is 93, which is great.

Locally-built website

/emodnet.wfs/docs/index.html

This is a detailed and well-organized introduction to the {emodnet.wfs} functions.
It is clearly written from the perspective of developers familiar with geospatial or ecological data.
The instructions are easy to follow.

Additional, less technical context would help new users orient themselves to working with this type of data.
An introduction could include things like

  • Explanations of the data related terms like "services," "layers," "features", "feature properties", how they relate to each other, and how they relate to more familiar data structures like data frames and columns.
  • The data accessed by this package is "geographic vector data." What is important about this format?
  • A less technical explanation of WFS that's more approachable than the technical documentaion provided.
  • @fmichonneau suggests displaying a map in the READme/this file.
  • @fmichonneau suggests recommends adding some context about what kinds of analyses the data sets created with this package are used for.
  • What constitutes an observation, and do observations line up across layers or features?

What does CRS stand for? What is the purpose of the transformation?

/emodnet.wfs/docs/articles/ecql_filtering.html

Line numbers in corresponding .Rmd file
line 330
starts "There is more that can accomplished"
should be "There is more that can be accomplished"

The paragraph at line 332 recommends using an alternative R package (eurobis) for large data sets.
Since filtering is possible with {emodnet.wfs}, it's not clear from the {eurobis} README why it's better? Is it because of the ability to filter by trait?

Using request parameters to limit query results

I was unable to confirm the map from

bbox_response %>%
  sf::st_cast(to = "MULTILINESTRING") %>%
  mapview::mapview(burst = TRUE, legend = FALSE)

but I did get links for zooming in and out and what sounded like a legend.

line 180
Under the heading for "Non-standard Vendor Parameters", there is a typo in the first line where "additional is missing an i.

Help pages

It's super helpful that the help pages are crosslinked, so the reader can put the function in it's context in the workflow
(like. the page for emodnet_get_layer_info explains that you would have used emodnet_get_wfs_info
to get layer names.)

It would be helpful to have more context again about vocabulary like "layers." "features".

The effect of reduce_layers would be clearer if I understood layers better and what is being combined.

In emodnet_get_layers, the advice about avoiding transferring unnecessary data by subsetting before downloading is great extra strategy that you don't often find in R documentation.

Are these different variables measured at common observation times?

For function emodnet.wfs, I'm confused about the term "feature services." Does "feature" mean "data" in this context, and is that different from the specific features that you can choose to retrieve?

The Value section of the help page for layer_atribute_inspect isn't really clear about what the inspection result is. Is it a more detailed documentation then get_layer_attribute_description? Or does it calculate summaries?

Code

The authors have much more experience with package development than I do, and the code looked great to me.

  • Variable and functions names had clear meanings.
  • Code was well organized within the R directory.
  • I didn't find any repetitious code.
  • Tasks were broken down into smaller functions that were easier to understand and test.
  • The style of the code was readable.

The list of services and URLs is provided as a .csv file with the package.
The authors have probably already thought of this and have a good reason,
like that a similar file is not available from the website.
I would be concerned that the package would need to be updated if new services were added or deleted from the server.

Summary

Congratulations to the authors on a great package that will be appreciated by researchers who will be able to easily access this complex data.
It would be helpful to have more documentation describing these complex data structures and the vocabulary used by the package,
both in the vignette/web site and in the help for the individual functions.

Thank you for this opportunity to study good code, learn a couple of new tips, and explore a package in a more broad way than I usually do in my work.

@MargaretSiple-NOAA
Copy link

Thank you for this thorough review @LizHareDogs !

@MargaretSiple-NOAA
Copy link

@ropensci-review-bot submit review #653 (comment) time 4.5

@ropensci-review-bot
Copy link
Collaborator

Logged review for robitalec (hours: 4.5)

@MargaretSiple-NOAA
Copy link

@ropensci-review-bot submit review #653 (comment) time 25

@ropensci-review-bot
Copy link
Collaborator

Logged review for LizHareDogs (hours: 25)

@MargaretSiple-NOAA
Copy link

Is there a way to give mentors in the champions program credit for participating in the review? @fmichonneau made good points that I'd like to add with credit to him.

Hi @LizHareDogs I apologize for not replying to this in the review thread. I could have sworn I wrote you something, but I can't find it! We do not have an official way here to provide credit as a reviewer aside from tagging them in the review thread (which you have done!), but another editor suggested that outside credit like a LinkedIn recommendation is also an appropriate way to record the work they put into mentoring you, and can also be helpful for them when future employers are looking at mentoring experience. I liked this blog post because it has concrete guidelines.

OK, now I am back to reading both of your reviews :)

@MargaretSiple-NOAA
Copy link

Hi @maelle - both reviews are in and have insightful comments; thank you both for that. The documentation suggestions from both reviewers will improve the accessibility of the package, especially for new users. I have just a couple small notes on naming conventions/documentation:

  • Regarding @robitalec 's suggestion about function names, I will leave it up to you whether you want to change the naming conventions for your functions. The current names are clear and sufficient, and @robitalec 's suggested conventions also make sense.
  • Regarding the function descriptions, I agree with both reviewers that the @description section can contain more detail and it would help the function documentation to distinguish the titles in the roxygen (for example, three different functions are titled with "Get WFS available layer information").

It is looking good so far and I will pass it off to you to address the reviewers' comments! ⚡

@maelle
Copy link
Member Author

maelle commented Nov 4, 2024

Thank you all for such helpful feedback!!

I'm just back from vacation, and will work on the suggestions ASAP (this week or next). Thank you!!

Regarding acknowledging all reviewers, we can list @fmichonneau along with @robitalec and @LizHareDogs in DESCRIPTION with the "rev" role.

@mpadge
Copy link
Member

mpadge commented Nov 5, 2024

@LizHareDogs Sorry about the error you're seeing with pkgcheck. We're aware of some path issues that will be fixed soon. Those are nevertheless primarily on Windows; issues with paths on Mac should be fixed. If you want to re-run the checks, you could try installing latest version of pkgcheck, which should also update pkgstats to latest. But only do that if you're interested, or have some other reason - there is no pressing need at all. Thanks!

@ropensci-review-bot
Copy link
Collaborator

@maelle, @salvafern, @annakrystalli: please post your response with @ropensci-review-bot submit response <url to issue comment> if you haven't done so already (this is an automatic reminder).

Here's the author guide for response. https://devguide.ropensci.org/authors-guide.html

@maelle
Copy link
Member Author

maelle commented Nov 12, 2024

👋 here! @annakrystalli, @salvafern and I are actively working on the package. We're meeting today, but still have a few things to cross off the list. Thank you for your understanding. 🙏

@maelle
Copy link
Member Author

maelle commented Nov 12, 2024

@MargaretSiple-NOAA It will take us a few more weeks to finish work on the package as the three of us are juggling other projects, but we have a plan. Should we put this issue on hold?

@MargaretSiple-NOAA
Copy link

@maelle Yes, no problem! I will tell the bot.

@MargaretSiple-NOAA
Copy link

@ropensci-review-bot put on hold

@ropensci-review-bot
Copy link
Collaborator

Submission on hold!

salvafern added a commit to EMODnet/EMODnet-Biology-Guidance that referenced this issue Nov 22, 2024
@maelle
Copy link
Member Author

maelle commented Dec 6, 2024

Thanks a lot to the reviewers @robitalec, @LizHareDogs, @fmichonneau and to the editor @MargaretSiple-NOAA for extremely useful feedback. We listed all reviewers in the DESCRIPTION file.

Response to @robitalec's review

Function naming

My related suggestion is to consider including a dictionary of this scheme somewhere in the README or introductory vignette. I am not sure exactly where/if this already exists in the broader EMODnet documentation but I think a description of each family of functions wfs/layer/attributes would be useful for new users getting started with the package.

We’ve decided to keep the current naming that we had chosen to align with the OGC standards. We’ve made many improvements to the documentation (README including heading titles, vignettes, manual pages) to ensure (hopefully!) the package becomes more user-friendly.

Function documentation

The documentation for this package is not extensively detailed. For example, the description sections for R functions are missing except for 1 function (emodnet_get_layers). I would suggest including a paragraph to describe each exported function so the user sees more than the title and list of parameters when they first open the help pages. I would also suggest including the name of the function in the parameters when an argument for a function is the output from another function, eg. in R/info.R: @param wfs …, from emodnet_init_wfs_client.

We have improved all manual pages, in particular we have made sure the titles of functions are more user-facing (with words like data sources, datasets, variables instead of web services, layers, attributes) with the description using the more specific words. We also improved the titles of sections on the pkgdown reference page. EMODnet/emodnet.wfs#177

Introductory vignette

I appreciated the suggestion from @MargaretSiple-NOAA but I think one step further for renaming the introductory vignette is to highlight that it is the introductory vignette with something like “getting started” or “introduction to” etc. At the moment, the introductory vignette on the pkgdown site is nestled between the other two vignettes making it unclear which vignette a user should start with.

We changed the title and position of the vignette in the navbar

The word “lot” is used a couple of times but it is unclear what this word refers to with respect to emodnet.wfs functions (wfs/layers/attributes).

We have rephrase the uses of “EMODnet lot” to “EMODnet thematic lot”. We have explained how EMODnet is organized in 7 thematic lots in the README.md and get started vignette. We have also added a new column to the emodnet_wfs() tibble emodnet_thematic_lot to specify the emodnet lot that manages the service.

Consider using {knitr} tables or similar functions to tidy up the printing of outputs.

We show data.frames as tibbles in https://emodnet.github.io/emodnet.wfs/articles/emodnet.wfs.html

This chunk feels like a pretty non-introductory chunk of code. Not sure if it's required or could be simplified.

We have simplified the code by requesting instead the data as geojson, and explained the reasoning for it. This is a known feature / issue of OGC services and we linked to sources explaining the feature.

There is an error related to mapview/webshot in the pkgdown/rendered vignette:

We fixed this bug.

Returned objects

The argument "reduce_layers" could use more explanation. The documentation suggests reducing layers will be attempted, but what is an example of reasons which reducing might fail?

Why does a single layer return a list? I think it's intuitive to match the names of multiple layers requested to a named list but wouldn't a user specifying a single layer expect an sf object to be directly returned?

We have renamed the argument to “simplify” to align it with base R functions, and we have clarified in the manual that it only works if column names are the same.

Minor comments

Given the overlap in authorship, if the authors are aware of how EMODnetWCS differs from emodnet.wfs or what the roadmap looks like for EMODnetWCS, it might be useful to include it in the README under other web services. Are these also additional tools to reference there? Could you add more precision to how "these three ways to access EMODnet complement each other"?

We have made the section about other packages more extensive: https://emodnet.github.io/emodnet.wfs/#unlock-the-full-potential-of-the-emodnet-web-services-access-raster-and-gridded-datasets

Codecov badge present in README twice, pointing to the root and tree/main, giving different code coverage percentages.

We removed the duplication.

The authors could use cffr::cff_write() to generate a citation file.

Now done.

Are there any rate limits/rules for the API? If so, could these be mentioned in the README and vignettes?

We have added the sentence “The use of the EMODnet Web Feature Services is not subjet to rate limiting at the moment.” to the README.

The skimr package is listed in the Suggests only to be used once in the ECQL filtering vignette. Is it a necessary dependency? I find reading through the vignette that the single skimr line returns an unexpected/different/confusing output to the previous outputs.

We ended up removing the line using skimr.

Some broken links detected with {urlchecker}.

We fixed the links in a few commits. Urlchecker now finds no broken link.

## Response to @LizHareDogs's review

README.md

Readthrough

From the perspective of someone who has not worked with this kind of data, it would be helpful if the introduction (which starts at line 21) could provide a little more context. What is geographic
vector data? I'm new to this kind of data so it would be helpful to know in a less technical way what it is.

We've added context and pre-requisites to the README.

Line 26
"though" should be "through"

We fixed the typo.

line 78
Not sure if I can use "View"

We replace the code with the sentence “To explore available services you can use View() or your usual way to explore data.frames.”

line 184
What does CRS stand for (it's mentioned a few times later)?

We added the definition in two vignettes.

Line 313
what is an sf object? What is the difference between this and how the data were retrieved before?

We added the context to the README in https://emodnet.github.io/emodnet.wfs/index.html#pre-requisites

Following instructions on Mac OS using Emacs, Emacs SPeaks Statistics

Line 78
I am not able to evaluate the View command on my system. This is not a problem with the package.
Otherwise, sample commands work as expected.

We have removed the exclusive mention of View() as a way to explore data.frames.

Vignette

All the commands in the vignette work for me.

Line 24 has the heading "data product." The meaning of this isn't clear to me. Will this be instructions on how to access a data product or how to create a data product?

We removed the confusing heading. “Data product” is an internal phrase for EMODnet team members, irrelevant for users of the package.

Line 109 has this command:
map <- mapview(habitats_directive_layers, zcol = "habitat_description", burst = TRUE)

I am able to run this command, which opens a browser. I can't check the map with the screen reader, but the text on the > screen seems appropriate.

In the rendered vignette at
the following error is displayed for the mapview command:

We have fixed the bug that lead to this chunk not working (we had not registered the
dependency on webshot).

Locally-built website

/emodnet.wfs/docs/index.html

This is a detailed and well-organized introduction to the {emodnet.wfs} functions.
It is clearly written from the perspective of developers familiar with geospatial or ecological data.
The instructions are easy to follow.

Additional, less technical context would help new users orient themselves to working with this type of data.
An introduction could include things like

  • Explanations of the data related terms like "services," "layers," "features", "feature properties", how they relate to each > other, and how they relate to more familiar data structures like data frames and columns.
  • The data accessed by this package is "geographic vector data." What is important about this format?
  • A less technical explanation of WFS that's more approachable than the technical documentaion provided.
  • @fmichonneau suggests displaying a map in the READme/this file.
  • @fmichonneau suggests recommends adding some context about what kinds of analyses the data sets created with
    this package are used for.
  • What constitutes an observation, and do observations line up across layers or features?

What does CRS stand for? What is the purpose of the transformation?

We have improved the manual pages, README, vignettes to ensure all terms like “layers” are accompanied by more beginner-friendly explanations. We have listed pre-requisites and resources in the README.
We prefer only showing a map in the introductory vignette.
We do not have published use cases yet but opened an issue to remind us to list them once available: EMODnet/emodnet.wfs#181

/emodnet.wfs/docs/articles/ecql_filtering.html

Line numbers in corresponding .Rmd file
line 330
starts "There is more that can accomplished"
should be "There is more that can be accomplished"

We fixed the typo.

The paragraph at line 332 recommends using an alternative R package (eurobis) for large data sets.
Since filtering is possible with {emodnet.wfs}, it's not clear from the {eurobis} README why it's better? Is it because of > the ability to filter by trait?

We have improved the filtering vignette with an explanation of why to use eurobis.

Using request parameters to limit query results

I was unable to confirm the map from
(...)
but I did get links for zooming in and out and what sounded like a legend.

line 180
Under the heading for "Non-standard Vendor Parameters", there is a typo in the first line where "additional is missing an i.

We fixed the typo.

Help pages

It's super helpful that the help pages are crosslinked, so the reader can put the function in it's context in the workflow
(like. the page for emodnet_get_layer_info explains that you would have used emodnet_get_wfs_info
to get layer names.)

It would be helpful to have more context again about vocabulary like "layers." "features".

We have improved help pages. In particular the reference index now features “easier” terms near the official terms: https://emodnet.github.io/emodnet.wfs/reference/index.html

The effect of reduce_layers would be clearer if I understood layers better and what is being combined.

We have renamed the parameter to simplify and improved its documentation.

In emodnet_get_layers, the advice about avoiding transferring unnecessary data by subsetting before downloading is great extra strategy that you don't often find in R documentation.

Are these different variables measured at common observation times?

In short: no. The only thing in common between the data available through this package is that they are in vector format, hence they can be points, lines, polygons etc. Depending on the dataset (layer) you are looking at, there might be a variable (attribute) that specifies time, but not all datasets (layer) will have a time dimension. In any case, when a time dimension is specified, EMODnet follows the ISO 8601 standard. Except if you are looking at multidimensional gridded data in a CF / COARDS compliant netcdf file, typically in ERDDAP, which the Unix Time format, normally as “days since January 1, 1970".

For function emodnet.wfs, I'm confused about the term "feature services." Does "feature" mean "data" in this context, and is that different from the specific features that you can choose to retrieve?

We have improved context in the README and headers of manual pages. Example: https://emodnet.github.io/emodnet.wfs/reference/emodnet_get_wfs_info.html

The Value section of the help page for layer_atribute_inspect isn't really clear about what the inspection result is. Is it a more detailed documentation then get_layer_attribute_description? Or does it calculate summaries?

We have added cross-references between manual pages of functions that help examine attributes, and we have made sure there are evaluated examples in the pkgdown manual pages.

Code

The authors have much more experience with package development than I do, and the code looked great to me.

  • Variable and functions names had clear meanings.
  • Code was well organized within the R directory.
  • I didn't find any repetitious code.
  • Tasks were broken down into smaller functions that were easier to understand and test.
  • The style of the code was readable.

The list of services and URLs is provided as a .csv file with the package.
The authors have probably already thought of this and have a good reason,
like that a similar file is not available from the website.
I would be concerned that the package would need to be updated if new services were added or deleted from the server.

We agree that it is a limitation, but before making a new service available to users of the package we would like to test it.

Conclusion (for now 😸 )

Thanks again to the reviewers and editor, we feel our package already became much stronger as a result of the feedback we received.

@maelle
Copy link
Member Author

maelle commented Dec 6, 2024

@ropensci-review-bot submit response #653 (comment)

@ropensci-review-bot
Copy link
Collaborator

Logged author response!

@maelle
Copy link
Member Author

maelle commented Dec 17, 2024

@MargaretSiple-NOAA could we please remove the holding label, since we've submitted our response? Thank you!

@MargaretSiple-NOAA
Copy link

@maelle Yes! Sorry! One sec.

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

8 participants