ropensci / software-review

rOpenSci Software Peer Review.
292 stars 104 forks source link

rWind: Download, Edit and Include Wind and Sea Currents Data in Ecological and Evolutionary Analysis #429

Closed jabiologo closed 2 years ago

jabiologo commented 3 years ago

Submitting Author Name: Javier Fernández López Submitting Author Github Handle: !--author1-->@jabiologo<!--end-author1-- Other Package Authors Github handles: (comma separated, delete if none) @KlausVigo, @PabloRL, Yurena Arjona Repository: https://github.com/jabiologo/rWind Version submitted: 1.1.6 Editor: @maurolepore Reviewers: TBD

Archive: TBD
Version accepted: TBD


Package: rWind
Encoding: UTF-8
Type: Package
Title: Download, Edit and Include Wind and Sea Currents Data in Ecological and Evolutionary Analysis
Version: 1.1.6
BugReports: https://github.com/jabiologo/rWind
Authors@R: c(person("Javier", "Fernández-López", email="jflopez.bio@gmail.com", role = c("aut", "cre")), 
    person("Klaus", "Schliep", email="klaus.schliep@gmail.com", role = c("aut")),
    person("Yurena", "Arjona", email="yurenaaf@gmail.com", role = c("aut")))
Maintainer: Javier Fernández-López <jflopez.bio@gmail.com>
Description: Tools for download and manage surface wind and sea currents data from the Global Forecasting System <https://www.ncdc.noaa.gov/data-access/model-data/model-datasets/global-forcast-system-gfs> and to compute connectivity between locations.
URL: http://allthiswasfield.blogspot.com.es/
License: GPL (>=3)
LazyData: TRUE
Imports: raster (>= 2.5-8), gdistance, Matrix, lubridate 
Suggests: testthat, rmarkdown, knitr
VignetteBuilder: knitr
Packaged: 2016-11-24 11:37:02 UTC; javi
Depends: R (>= 3.4)
NeedsCompilation: no
Repository: none
RoxygenNote: 7.1.1

Scope

There are other packages that covers data download and data managing that could overlap with rWind. However, rWind is specially focused to offer to the users a straightforward workflow from data download to cost analysis between locations. Some identified packages that cover partially the same utilities as rWind are:

rnoaa: Client for many 'NOAA' data sources including the 'NCDC' climate 'API', with functions foreach of the 'API' 'endpoints': data, data categories, data sets, data types,locations, location categories, and stations. This package allows the access of a huge NOAA data sources, but is not focused in transform data into raster format, nor in cost analyses. In addition, the high amount of data sources makes not easy for a non specialized user to decide what data base should be used for each analyses. It has not "official" vignettes at https://cran.r-project.org/web/packages/rnoaa/index.html

RNCEP: This package, similarly as the previous one, contains functions to retrieve, organize, and visualize weather data from the NCEP/NCAR Reanalysis, and NCEP/DOE Reanalysis II datasets. As the precious one, it is focused into obtain, manage (aggregate, etc) and visualize a huge amount of weather data. Visualization (NCEP.vis.area) are plot-based, non raster layers, and conversion among formats is not addressed in the package. Cost analyses are not implemented. It has not "official" vignettes at https://cran.r-project.org/web/packages/RNCEP/index.html. Kemp et al (2012)

weathercran: This package is developed to search for and download multiple months/years of historical weather data from Environment and Climate Change Canada (ECCC) website. Data bases are different to those used by rWind and data managing and cost analysis computations are not covered in this package. It has many "official" vignettes at https://cran.r-project.org/web/packages/weathercan/index.html.

In short, rWind is focused to fill the gap between wind and ocean currents data adquisition/managing and cost analysis among locations. It provides a straightforward framework to accomplish all these processes in a single R package.

References Felicísimo, Á. M., Muñoz, J., & González-Solis, J. (2008). Ocean surface winds drive dynamics of transoceanic aerial movements. PLoS One, 3(8), e2928. Fernández‐López, J., & Schliep, K. (2019). rWind: download, edit and include wind data in ecological and evolutionary analysis. Ecography, 42(4). https://onlinelibrary.wiley.com/doi/full/10.1111/ecog.03730 https://doi.org/10.1111/ecog.03730 Kemp, M. U., Emiel van Loon, E., Shamoun‐Baranes, J., & Bouten, W. (2012). RNCEP: global weather and climate data at your fingertips. Methods in Ecology and Evolution, 3(1), 65-70. https://besjournals.onlinelibrary.wiley.com/doi/10.1111/j.2041-210X.2011.00138.x

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

melvidoni commented 3 years ago

Hello @jabiologo, thanks for the submission.

Before we consider the package, we would like you to elaborate on the overlap of rWind with other packages. Please, edit your post and be as thorough as possible. Please let me know once it is done.

melvidoni commented 3 years ago

Hello @jabiologo , please extend the explanation as requested so we can move forward.

jabiologo commented 3 years ago

Hi @melvidoni , sorry about the delay. Please, find the edited post above and let me know any questions you have. Thanks in advance.

melvidoni commented 3 years ago

Thank you for the corrections. @maurolepore will be the Handling Editor.

maurolepore commented 3 years ago

Hi @jabiologo! It's my pleasure to edit your submission. I'm about to start editor checks. You'll here back from me within the next couple of days.

maurolepore commented 3 years ago

@jabiologo, here are the editor checks. Please respond to the comments prefixed with "(ml##)" and consider the other comments optional for now. Once you address the prefixed comments I'll start looking for reviewers.

Editor checks:


Editor comments

Documentation > README

Automated checks:

``` r spelling::spell_check_package() #> DESCRIPTION does not contain 'Language' field. Defaulting to 'en-US'. #> WORD FOUND IN #> Á flow.dispersion_int.Rd:61 #> flow.dispersion.Rd:77 #> adviced flow.dispersion_int.Rd:46 #> flow.dispersion.Rd:60 #> al flow.dispersion_int.Rd:14,35,42 #> flow.dispersion.Rd:28,49,56 #> allthiswasfield rWind-package.Rd:17 #> wind.data.Rd:19 #> wind.series.Rd:20 #> anisotropic flow.dispersion_int.Rd:27 #> flow.dispersion.Rd:41 #> Shortest_paths.Rmd:41 #> Anisotropic Shortest_paths.Rmd:2 #> arrowDir arrowDir.Rd:17,22,27 #> asimetric flow.dispersion_int.Rd:28 #> flow.dispersion.Rd:42 #> betwen flow.dispersion_int.Rd:25 #> flow.dispersion.Rd:39 #> blogspot rWind-package.Rd:17 #> wind.data.Rd:19 #> wind.series.Rd:20 #> coastwatch seaOscar.dl.Rd:47,70 #> README.md:17 #> codecov README.md:8 #> colection wind.dl_2.Rd:48 #> wind.dl.Rd:60 #> componenats wind.mean.Rd:22 #> coordenate seaOscar.dl.Rd:42 #> wind.dl_2.Rd:40,65 #> wind.dl.Rd:52 #> coordenates wind.data.Rd:23 #> wind.dl_2.Rd:65 #> wind.dl.Rd:72 #> CRS wind2raster_int.Rd:25 #> wind2raster.Rd:25 #> csv seaOscar.dl.Rd:35,36,41,53,54 #> wind.dl_2.Rd:27,28,39,56,57 #> wind.dl.Rd:43,44,48,51,66,67 #> databse seaOscar.dl.Rd:50 #> DCL seaOscar.dl.Rd:68 #> wind.dl_2.Rd:88 #> wind.dl.Rd:86 #> dgCMatrix flow.dispersion_int.Rd:17 #> flow.dispersion.Rd:31 #> diferent flow.dispersion_int.Rd:16 #> flow.dispersion.Rd:30 #> dir arrowDir.Rd:11 #> uv2ds.Rd:30 #> wind.dl_2.Rd:67 #> wind.dl.Rd:73 #> wind2raster_int.Rd:21 #> directionality flow.dispersion_int.Rd:27 #> flow.dispersion.Rd:41 #> dl seaOscar.dl.Rd:35,46 #> wind.data.Rd:22 #> wind.dl_2.Rd:27,44,54,55,62 #> wind.dl.Rd:43,56 #> wind.fit_int.Rd:10,16,23 #> wind.series.Rd:23 #> wind2raster.Rd:21 #> doi flow.dispersion_int.Rd:67 #> flow.dispersion.Rd:83 #> dplyr tidy.rWind_series.Rd:19 #> ds uv2ds.Rd:22,24 #> edu wind.data.Rd:37 #> wind.dl_2.Rd:51,90 #> wind.dl.Rd:63,88 #> wind.series.Rd:41 #> erddap seaOscar.dl.Rd:47,70 #> wind.data.Rd:37 #> wind.dl_2.Rd:51,90 #> wind.dl.Rd:63,88 #> wind.series.Rd:41 #> README.md:17 #> et flow.dispersion_int.Rd:14,35,42 #> flow.dispersion.Rd:28,49,56 #> Etten flow.dispersion_int.Rd:65 #> flow.dispersion.Rd:81 #> Felicísimo flow.dispersion_int.Rd:14,42,61 #> flow.dispersion.Rd:28,56,77 #> Felícisimo flow.dispersion_int.Rd:35 #> flow.dispersion.Rd:49 #> Fernández arrowDir.Rd:56 #> flow.dispersion_int.Rd:73 #> flow.dispersion.Rd:89 #> rWind-package.Rd:20,26 #> seaOscar.dl.Rd:76 #> uv2ds.Rd:42 #> wind.dl_2.Rd:97 #> wind.dl.Rd:94 #> wind.fit_int.Rd:33 #> wind.mean.Rd:39 #> wind2raster_int.Rd:32 #> wind2raster.Rd:41 #> forcast wind.dl_2.Rd:46 #> wind.dl.Rd:58 #> README.md:13 #> gdistance flow.dispersion_int.Rd:19,29,65 #> flow.dispersion.Rd:33,43,81 #> Geospatial seaOscar.dl.Rd:48 #> wind.dl_2.Rd:48 #> wind.dl.Rd:60 #> gfs wind.dl_2.Rd:46 #> wind.dl.Rd:58 #> README.md:13 #> GFS rWind-package.Rd:10 #> wind.dl_2.Rd:15,45,47,66 #> wind.dl.Rd:30,56,59,72 #> wind.fit_int.Rd:17,19 #> ggplot tidy.rWind_series.Rd:19 #> ggvis tidy.rWind_series.Rd:19 #> gis seaOscar.dl.Rd:68 #> wind.dl_2.Rd:88 #> wind.dl.Rd:86 #> gmail rWind-package.Rd:26 #> seaOscar.dl.Rd:76 #> uv2ds.Rd:42 #> wind.dl_2.Rd:97 #> wind.dl.Rd:94 #> wind.fit_int.Rd:33 #> wind.mean.Rd:39 #> wind2raster_int.Rd:32 #> wind2raster.Rd:41 #> González flow.dispersion_int.Rd:61 #> flow.dispersion.Rd:77 #> griddap wind.dl_2.Rd:90 #> wind.dl.Rd:63,88 #> gridded wind2raster_int.Rd:17 #> wind2raster.Rd:17 #> hawaii wind.data.Rd:37 #> wind.dl_2.Rd:51,90 #> wind.dl.Rd:63,88 #> wind.series.Rd:41 #> HorizontalFactor flow.dispersion_int.Rd:39,41 #> flow.dispersion.Rd:53,55 #> http rWind-package.Rd:17 #> seaOscar.dl.Rd:68 #> wind.data.Rd:19,37 #> wind.dl_2.Rd:51,88,90 #> wind.dl.Rd:86 #> wind.series.Rd:20,41 #> README.md:15 #> https arrowDir.Rd:50 #> seaOscar.dl.Rd:47,70 #> wind.dl_2.Rd:46 #> wind.dl.Rd:58,63,88 #> wind.fit_int.Rd:26 #> wind.mean.Rd:33 #> README.md:13,17 #> incrementaly flow.dispersion_int.Rd:41 #> flow.dispersion.Rd:55 #> internaly wind.dl_2.Rd:66 #> wind.dl.Rd:73 #> wind.fit_int.Rd:16,23 #> jflopez rWind-package.Rd:26 #> seaOscar.dl.Rd:76 #> uv2ds.Rd:42 #> wind.dl_2.Rd:97 #> wind.dl.Rd:94 #> wind.fit_int.Rd:33 #> wind.mean.Rd:39 #> wind2raster_int.Rd:32 #> wind2raster.Rd:41 #> jplOscar seaOscar.dl.Rd:47,70 #> README.md:17 #> jss flow.dispersion_int.Rd:67 #> flow.dispersion.Rd:83 #> Karline arrowDir.Rd:48 #> logitude wind2raster_int.Rd:18 #> wind2raster.Rd:19 #> lon seaOscar.dl.Rd:43 #> wind.dl_2.Rd:41 #> wind.dl.Rd:53 #> LonPM seaOscar.dl.Rd:47,70 #> README.md:17 #> López arrowDir.Rd:56 #> flow.dispersion_int.Rd:73 #> flow.dispersion.Rd:89 #> rWind-package.Rd:20,26 #> seaOscar.dl.Rd:76 #> uv2ds.Rd:42 #> wind.dl_2.Rd:97 #> wind.dl.Rd:94 #> wind.fit_int.Rd:33 #> wind.mean.Rd:39 #> wind2raster_int.Rd:32 #> wind2raster.Rd:41 #> Muñoz flow.dispersion_int.Rd:61 #> flow.dispersion.Rd:77 #> ncdc wind.dl_2.Rd:46 #> wind.dl.Rd:58 #> README.md:13 #> ncep wind.dl.Rd:63,88 #> NCEP wind.data.Rd:37 #> wind.dl_2.Rd:47,51,90 #> wind.dl.Rd:59 #> wind.series.Rd:41 #> noaa seaOscar.dl.Rd:47,70 #> wind.dl_2.Rd:46 #> wind.dl.Rd:58 #> README.md:13,17 #> NOAA wind.dl_2.Rd:47 #> wind.dl.Rd:59 #> nocite Shortest_paths.Rmd:14 #> NWS wind.dl_2.Rd:45 #> wind.dl.Rd:57 #> oos wind.data.Rd:37 #> wind.dl_2.Rd:51,90 #> wind.series.Rd:41 #> oscar seaOscar.dl.Rd:36 #> pacioos wind.dl.Rd:63,88 #> pae wind.dl.Rd:63,88 #> paha wind.dl.Rd:63,88 #> pfeg seaOscar.dl.Rd:47,70 #> README.md:17 #> PLoS flow.dispersion_int.Rd:62 #> flow.dispersion.Rd:78 #> POSIXt wind.dl_2.Rd:13 #> procesed uv2ds.Rd:30 #> rasterFromXYZ wind2raster_int.Rd:19 #> wind2raster.Rd:20 #> RasterStack flow.dispersion_int.Rd:10 #> flow.dispersion.Rd:24 #> rwind wind.data.Rd:19 #> wind.series.Rd:20 #> seaOscar seaOscar.dl.Rd:35,46 #> skalar flow.dispersion.Rd:13,15 #> soest wind.data.Rd:37 #> wind.dl_2.Rd:51,90 #> wind.series.Rd:41 #> Soetaert arrowDir.Rd:48 #> tendences wind.dl_2.Rd:64 #> th Shortest_paths.Rmd:46 #> transfored wind.dl_2.Rd:14 #> transitionLayer flow.dispersion_int.Rd:18,24 #> flow.dispersion.Rd:32,38 #> TransitionLayer flow.dispersion_int.Rd:18,29 #> flow.dispersion.Rd:32,43 #> tt wind.dl_2.Rd:28,57 #> wind.dl.Rd:44,67 #> uv uv2ds.Rd:22,24 #> WDOWmbV seaOscar.dl.Rd:68 #> wind.dl_2.Rd:88 #> wind.dl.Rd:86 #> WGS wind2raster_int.Rd:25 #> wind2raster.Rd:25 #> wikipedia wind.fit_int.Rd:26 #> wind.mean.Rd:33 #> www seaOscar.dl.Rd:68 #> wind.dl_2.Rd:46,88 #> wind.dl.Rd:58,86 #> README.md:13,15 #> yyyy seaOscar.dl.Rd:36,54 #> wind.dl_2.Rd:28,57 #> wind.dl.Rd:44,67 ```
``` r goodpractice::gp() #> Preparing: covr #> Preparing: cyclocomp #> #> checking for file ‘/tmp/Rtmp3bi5u3/remotes96556a3cb92/rWind/DESCRIPTION’ ... ✓ checking for file ‘/tmp/Rtmp3bi5u3/remotes96556a3cb92/rWind/DESCRIPTION’ #> ─ preparing ‘rWind’: #> ✓ checking DESCRIPTION meta-information #> checking vignette meta-information ... ✓ checking vignette meta-information #> ─ checking for LF line-endings in source and make files and shell scripts #> ─ checking for empty or unneeded directories #> ─ building ‘rWind_1.1.6.tar.gz’ #> #> #> Preparing: description #> Preparing: lintr #> Preparing: namespace #> Preparing: rcmdcheck #> ── GP rWind ──────────────────────────────────────────────────────────────────── #> #> It is good practice to #> #> ✖ write unit tests for all functions, and all package code in #> general. 36% of code lines are covered by test cases. #> #> R/wind_functions2.R:81:NA #> R/wind_functions2.R:83:NA #> R/wind_functions2.R:84:NA #> R/wind_functions2.R:85:NA #> R/wind_functions2.R:88:NA #> ... and 232 more lines #> #> ✖ avoid long code lines, it is bad for readability. Also, many people #> prefer editor windows that are about 80 characters wide. Try make #> your lines shorter than 80 characters #> #> R/rWind-package.R:3:1 #> R/rWind-package.R:5:1 #> R/rWind-package.R:6:1 #> R/wind_functions2.R:27:1 #> R/wind_functions2.R:115:1 #> ... and 31 more lines #> #> ──────────────────────────────────────────────────────────────────────────────── ```
# bash
ls -l *.Rproj
$ -rwxrwxr-x 1 rstudio rstudio 356 Mar 13 08:29 rWind1.Rproj
$ -rw-rw-r-- 1 rstudio rstudio 343 Mar 13 20:34 rWind.Rproj

Reviewers: TBC
Due date: TBC

maurolepore commented 3 years ago

Hello @jabiologo, How are things going? Do you need more time or help?

jabiologo commented 3 years ago

Hi @maurolepore thanks for contacting. My colleague Klaus already did some changes:

-> remove travisCI, fix 1 url
-> add github actions for checks
-> remove redundant file
-> fix some spelling mistakes
-> clean up DESCRIPTION file
-> bug fix
-> add test again

but I was too busy with some deadlines and I'd need more time to finish some more things and respond the comments. Could I have one more week? Thanks in advance.

maurolepore commented 3 years ago

Sure. I mostly wanted to touch base to ensure you have the resources you need. You seem to be in good track but let me know otherwise.

maurolepore commented 3 years ago

Hi @jabiologo, are you still good to address this issues soon or do you prefer to put the submission on hold until you are ready?

jabiologo commented 3 years ago

Hi @maurolepore, sorry for the delay and thanks for your constructive commentaries. We addressed the most of the suggestion you made, please find below a detailed list with some responses:

Documentation > README

(ml01) rWind currently works with just two databases, one for wind data and other for sea currents data, one of the URLs was duplicated since it referred to the same database. We have corrected this.

(ml02) We have added more detailed description of the package as well as the reference to the peer reviewed published paper about rWind for more information.

. We have added one simple example from the vignette into the README file.

. We have added some statements about differences and improvements over other packages.

. We have removed the “License” section

Automated checks:

(ml03) Spell check has been conducted and word list has been updated (ml04) goodpractice::gc() has been run. It suggested two main issues: only 41% of our code is currently tested. Current time availability doesn’t allow us to afford new test… The other main issue was related to too long code lines. Those lines are the URLs used by rWind to connect to different data bases to download wind and sea currents data, so we consider it makes sense to maintain as they are. . Travis file has been removed and GitHub actions are now used . The package tests are now conducted in Windows, iOS and Linux (ml05) GitHub actions are now used to test the package . use_tidy_style() done and updated . rWind1.Rproj has been removed

DESCRIPTION

. urlchecker::url_update() has been run. It reports an issue related to https://www.ncdc.noaa.gov/data-access/model-data/model-datasets/global-forcast-system-gfs. However, we think it is due to the way the original server is managing the query for this URL. . usethis issues have been addressed . snake_case issue: rWind is already in wide use, the paper has been cited at least 19 times in other peer reviewed papers and many blog post and tweets have been cited. In any case, we can consider in a near future to make something similar as the package igraph, in which they maintain both, original and snake_case functions. Could this work for you? . We’ll consider the "R/f.R" and "tests/testthat/test-f.R" structure in near future. . usethis::use_roxygen_md() done

Thanks again for your work.

maurolepore commented 3 years ago

@jabiologo,

Thanks for addressing most of my most pressing concerns.

(ml04) ... only 41% of our code is currently tested. Current time availability doesn’t allow us to afford new test.

Our guide states that Test coverage below 75% will likely require additional tests or explanation before being sent for review. Although test coverage near 100% is a poor predictor of high-quality code, low test coverage is a good predictor of poor quality code (Unit testing principles, practices, and patterns).

Increasing test coverage might not be too time consuming and will likely pay off. If appropriate you may exclude specific chunks of code with an explanatory comment. And if you haven't seen them yet, check rOpenSci's resources on testing R packages, and let me know if you get stuck.

I totally understand you are busy. It happens and you should not worry. Would you like to give it a go or prefer to hold?

(The hold has no impact other than freeing me to help review other packages)

jabiologo commented 3 years ago

Hi @maurolepore

Thanks very much for your nice comments. I was talking with my friend and we think it would be a good idea to hold the submission to free you to help with other packages. In the meanwhile we could address the testing issue. What would be the procedure to restart the submission?

Thanks in advance, best

maurolepore commented 3 years ago

Thanks @jabiologo. Whenever you are ready, add a comment here saying so. Then I'll remove the holding label and start searching for reviewers. "The holding status will be revisited every 3 months, and after one year the issue will be closed".

maurolepore commented 3 years ago

Hi @jabiologo, it's about the time when we should review the "holding" status. Shall we keep it?

--

The author can choose to have their submission put on hold (editor applies the holding label). The holding status will be revisited every 3 months, and after one year the issue will be closed. -- -- https://devdevguide.netlify.app/policies.html#policiesreviewprocess

maurolepore commented 3 years ago

Hi @jabiologo, it's time to remind you that this package continues on hold. Let me know whenever you're ready to proceed with the review. Else I'll touch base again around Jan 30, 2022, and if the hold continues I'll close this issue on April 30, 2022.

--

The author can choose to have their submission put on hold (editor applies the holding label). The holding status will be revisited every 3 months, and after one year the issue will be closed. -- -- https://devdevguide.netlify.app/policies.html#policiesreviewprocess

maurolepore commented 2 years ago

Hi @jabiologo, it's time to remind you that this package continues on hold. Let me know whenever you're ready to proceed with the review. Else I'll close this issue on April 30, 2022.

--

The author can choose to have their submission put on hold (editor applies the holding label). The holding status will be revisited every 3 months, and after one year the issue will be closed. -- https://devdevguide.netlify.app/policies.html#policiesreviewprocess

maurolepore commented 2 years ago

Hi @jabiologo, I'm closing this issue now because it's been on hold for more than 1 year.

--

The author can choose to have their submission put on hold (editor applies the holding label). The holding status will be revisited every 3 months, and after one year the issue will be closed. -- https://devdevguide.netlify.app/policies.html#policiesreviewprocess