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

dendroNetwork #627

Closed
14 of 29 tasks
RonaldVisser opened this issue Feb 8, 2024 · 50 comments
Closed
14 of 29 tasks

dendroNetwork #627

RonaldVisser opened this issue Feb 8, 2024 · 50 comments
Assignees

Comments

@RonaldVisser
Copy link

RonaldVisser commented Feb 8, 2024

Date accepted: 2024-04-25

Submitting Author Name: Ronald M. Visser
Submitting Author Github Handle: @RonaldVisser
Repository: https://github.com/RonaldVisser/dendroNetwork
Version submitted: 0.5.0
Submission type: Standard
Editor: @ldecicco-USGS
Reviewers: @kaijagahm, @gzach93

Archive: TBD
Version accepted: TBD
Language: en


  • Paste the full DESCRIPTION file inside a code block below:
Package: dendroNetwork
Type: Package
Title: Create networks of dendrochronological series using pairwise similarity
Version: 0.5.0
Authors@R: 
    c(person(given ="Ronald", 
            family = "Visser", 
            email = "r.m.visser@saxion.nl", 
            role = c("aut", "cre"),
            comment = c(ORCID = "0000-0001-6966-1729")),
    person(given = "Angelino", 
            family ="Salentino",
            role = "ctb",
            comment = c(ORCID = "0000-0002-4763-3943")),
   person(given = "Andy", 
            family ="Bunn",
            role = "ctb",
            comment = c(ORCID = "0000-0001-9027-2162")))
Maintainer: Ronald Visser <r.m.visser@saxion.nl>
Depends: R (>= 3.5.0)
Imports: 
    dplR (>= 1.7.2),
    igraph,
    stringr,
    reshape2,
    RCy3,
    dplyr,
    RColorBrewer,
    tidyr,
    foreach,
    magrittr,
    doParallel,
    lifecycle, 
    stats,
    grDevices
Description: This package makes it easy to create dendrochronological networks based on the similarity between tree-ring series or chronologies. It includes various functions to compare tree-ring curves building upon dplR. The networks can be used to visualise and understand the relations between tree-ring curves. These networks are also very useful to estimate the provenance or wood-use within a structure/context/site.
License: GPL (>= 3)
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.3.1
Suggests: 
    testthat (>= 3.0.0)
Config/testthat/edition: 3
Roxygen: list(markdown = TRUE)

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 software deals with the analysis of dendrochronological data and aims the help to study, understand and visualize patterns in the data using network analyses. The software combines R with Cytoscape.

  • Who is the target audience and what are scientific applications of this package?
    Dendrochronologist. The scientific application lies in the analyses and visualization of large datasets using networks. The method has been applied before to determine the provenance of wood from archaeological contexts, but can also be used to determine other patterns based on the similarity between tree-ring series or even other time series.

  • Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?

  • No, none as far as I know.

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

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

I tried to run pkgcheck, but it failed with the following feedback:

Error in vapply(tag_data, function(i) i$keyword, character(1L)) :
values must be length 1,
but FUN(X[[12]]) result is length 2

I have absolutely no idea why....

The same error occurs locally and using github action.

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

Note: The following R packages were unable to be installed/upgraded on our system: [RCy3]; some checks may be unreliable.

@ldecicco-USGS
Copy link

I'm guessing the issue is that RCy3 is only on Bioconductor. The mentions I found in the rOpenSci guide:

https://devguide.ropensci.org/building.html?q=Biocond#pkgdependencies

And:
"If you develop a package depending on or intended for Bioconductor, you might find biocthis relevant."

I'll poke around more and see if I can find other recent submissions that used Bioconductor packages.

@mpadge
Copy link
Member

mpadge commented Feb 9, 2024

Sorry @ldecicco-USGS and @RonaldVisser, that was a bug with Bioc installs on our side. Checks should appear in a moment.

@ropensci-review-bot
Copy link
Collaborator

Checks for dendroNetwork (v0.5.0)

git hash: 02a3fe68

  • ✔️ Package name is available
  • ✖️ does not have a 'codemeta.json' file.
  • ✖️ does not have a 'contributing' file.
  • ✔️ uses 'roxygen2'.
  • ✖️ 'DESCRIPTION' does not have a URL field.
  • ✖️ 'DESCRIPTION' does not have a BugReports field.
  • ✔️ Package has at least one HTML vignette
  • ✔️ All functions have examples.
  • ✖️ Continuous integration checks unavailable (no URL in 'DESCRIPTION').
  • ✖️ Package coverage failed
  • ✖️ R CMD check process failed with message: 'Build process failed'.
  • 👀 Function names are duplicated in other packages

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

(Checks marked with 👀 may be optionally addressed.)

Package License: GPL (>= 3)


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 base 164
internal dendroNetwork 20
imports igraph 25
imports magrittr 20
imports reshape2 10
imports RCy3 9
imports dplyr 7
imports stats 6
imports grDevices 4
imports RColorBrewer 3
imports dplR 2
imports foreach 2
imports doParallel 1
imports stringr NA
imports tidyr NA
imports lifecycle NA
suggests knitr NA
suggests rmarkdown NA
suggests testthat 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.

base

length (20), lapply (10), subset (10), unlist (10), max (9), for (8), min (8), as.vector (6), c (6), matrix (5), ncol (5), apply (4), as.numeric (4), rownames (4), by (3), cbind (3), colSums (3), dim (3), formatC (3), merge (3), nchar (3), nrow (3), paste0 (3), sqrt (3), system.file (3), unique (3), diff (2), is.na (2), mean (2), rep (2), seq (2), seq_along (2), abs (1), all.vars (1), as.data.frame (1), list (1), log (1), match.call (1), sum (1)

igraph

cliques (6), add_edges (4), make_empty_graph (4), V (3), E (2), graph.data.frame (2), clique_num (1), cluster_edge_betweenness (1), decompose (1), decompose.graph (1)

dendroNetwork

cor_mat_overlap (3), t_value (3), wuchswerte (3), clique_community_names_par (2), sim_table (2), clique_community_names (1), cyto_clean_styles (1), cyto_create_cpm_style (1), cyto_create_gn_style (1), cyto_create_graph (1), dendro_network (1), find_all_cpm_com (1)

magrittr

%>% (20)

reshape2

melt (10)

RCy3

createNetworkFromIgraph (4), importVisualStyles (3), layoutNetwork (1), loadTableData (1)

dplyr

mutate (5), row_number (2)

stats

D (3), weights (2), pnorm (1)

grDevices

colorRampPalette (2), colors (2)

RColorBrewer

brewer.pal (3)

dplR

as.rwl (2)

foreach

foreach (2)

doParallel

registerDoParallel (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 16 files) and
  • 1 authors
  • 1 vignette
  • 2 internal data files
  • 14 imported packages
  • 13 exported functions (median 29 lines of code)
  • 13 non-exported functions in R (median 28 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 16 74.9
files_vignettes 3 92.4
files_tests 3 75.2
loc_R 434 43.1
loc_vignettes 161 41.5
loc_tests 12 9.0
num_vignettes 1 64.8
data_size_total 112963 84.1
data_size_median 56481 88.8
n_fns_r 26 35.7
n_fns_r_exported 13 53.9
n_fns_r_not_exported 13 28.8
n_fns_per_file_r 1 0.2 TRUE
num_params_per_fn 3 33.6
loc_per_fn_r 28 74.9
loc_per_fn_r_exp 29 61.6
loc_per_fn_r_not_exp 28 76.0
rel_whitespace_R 5 19.2
rel_whitespace_vignettes 21 25.8
rel_whitespace_tests 17 5.2
doclines_per_fn_exp 31 34.8
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 7 27.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)


3b. goodpractice results

R CMD check with rcmdcheck

R CMD check generated the following error:

  1. Error in proc$get_built_file() : Build process failed

R CMD check generated the following check_fails:

  1. description_url
  2. description_bugreports

Test coverage with covr

ERROR: Test Coverage Failed

Cyclocomplexity with cyclocomp

Error : Build failed, unknown error, standard output:
�[33m* checking for file ‘dendroNetwork/DESCRIPTION’ ... OK

  • preparing ‘dendroNetwork’:
  • checking DESCRIPTION meta-information ... OK
  • installing the package to build vignettes
  • creating vignettes ... ERROR
    --- re-building ‘dendroNetwork_use.Rmd’ using rmarkdown

Quitting from lines 21-44 [flowchart_workflow] (dendroNetwork_use.Rmd)
Error: processing vignette 'dendroNetwork_use.Rmd' failed with diagnostics:
there is no package called 'DiagrammeR'
--- failed re-building ‘dendroNetwork_use.Rmd’

SUMMARY: processing the following file failed:
‘dendroNetwork_use.Rmd’

Error: Vignette re-building failed.
Execution halted
�[39m

Static code analyses with lintr

lintr found the following 89 potential issues:

message number of times
Avoid 1:length(...) expressions, use seq_len. 2
Avoid library() and require() calls in packages 2
Lines should not be more than 80 characters. 85


4. Other Checks

Details of other checks (click to open)

✖️ The following function name is duplicated in other packages:

    • sim_table from BCEA


Package Versions

package version
pkgstats 0.1.3.11
pkgcheck 0.1.2.15


Editor-in-Chief Instructions:

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

@ldecicco-USGS
Copy link

Thanks for the submission @RonaldVisser ! Looks like a neat package.

Before we can proceed you'll need to get continuous integration set up. I'd start by trying:
https://usethis.r-lib.org/reference/github_actions.html#use-github-action-check-standard-

That might work - I haven't used Bioconductor packages in awhile, so I'm not 100% sure. If that doesn't work, then you could try:
https://rdrr.io/bioc/biocthis/man/use_bioc_github_action.html

The other "X" items above should be pretty straightforward to address, but let me know if you have any questions.

The error on the build is:

Quitting from lines 21-44 [flowchart_workflow] (dendroNetwork_use.Rmd)
Error: processing vignette 'dendroNetwork_use.Rmd' failed with diagnostics:
there is no package called 'DiagrammeR'
--- failed re-building ‘dendroNetwork_use.Rmd’

If DiagrammeR isn't used in any of the package functions, you'll want to add it to "Suggests" for that vignette to be generated.

@RonaldVisser
Copy link
Author

Thanks for the compliment! Thank you for the comments and help!

I have done some updating of various functions, added some tests and the continuous integration (github actions R-CMD / pkgcheck) seems to be working fine. According to pkgcheck it now checks all the boxes :)

Due to the various edits, I released a new release (0.5.1), with changes described in the NEWS-file.

@ldecicco-USGS
Copy link

@ropensci-review-bot check package

@ropensci-review-bot
Copy link
Collaborator

Thanks, about to send the query.

@ropensci-review-bot
Copy link
Collaborator

🚀

Editor check started

👋

@mpadge
Copy link
Member

mpadge commented Feb 14, 2024

Apologies once again @ldecicco-USGS @RonaldVisser. Failed this time because the checks appear to have been called at exactly the time the system was scheduled for regular rebuild (first few hours of Tues, UTC). I've just submitted a PR to our DevGuide to clarify that. Checks will appear straight after this.

@ropensci-review-bot
Copy link
Collaborator

Checks for dendroNetwork (v0.5.2)

git hash: e76e3dd1

  • ✔️ 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 91.4%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.
  • 👀 Function names are duplicated in other packages

(Checks marked with 👀 may be optionally addressed.)

Package License: GPL (>= 3)


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 base 164
internal dendroNetwork 20
imports igraph 25
imports magrittr 20
imports reshape2 10
imports RCy3 9
imports dplyr 7
imports stats 6
imports grDevices 4
imports RColorBrewer 3
imports dplR 2
imports foreach 2
imports doParallel 1
imports stringr NA
imports tidyr NA
imports lifecycle NA
suggests knitr NA
suggests rmarkdown NA
suggests testthat NA
suggests DiagrammeR 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.

base

length (20), lapply (10), subset (10), unlist (10), max (9), for (8), min (8), as.vector (6), c (6), matrix (5), ncol (5), apply (4), as.numeric (4), rownames (4), by (3), cbind (3), colSums (3), dim (3), formatC (3), merge (3), nchar (3), nrow (3), paste0 (3), sqrt (3), system.file (3), unique (3), diff (2), is.na (2), mean (2), rep (2), seq (2), seq_along (2), abs (1), all.vars (1), as.data.frame (1), list (1), log (1), match.call (1), sum (1)

igraph

cliques (6), add_edges (4), make_empty_graph (4), V (3), decompose (2), E (2), graph_from_data_frame (2), clique_num (1), cluster_edge_betweenness (1)

dendroNetwork

cor_mat_overlap (3), t_value (3), wuchswerte (3), clique_community_names_par (2), sim_table (2), clique_community_names (1), cyto_clean_styles (1), cyto_create_cpm_style (1), cyto_create_gn_style (1), cyto_create_graph (1), dendro_network (1), find_all_cpm_com (1)

magrittr

%>% (20)

reshape2

melt (10)

RCy3

createNetworkFromIgraph (4), importVisualStyles (3), layoutNetwork (1), loadTableData (1)

dplyr

mutate (5), row_number (2)

stats

D (3), weights (2), pnorm (1)

grDevices

colorRampPalette (2), colors (2)

RColorBrewer

brewer.pal (3)

dplR

as.rwl (2)

foreach

foreach (2)

doParallel

registerDoParallel (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 16 files) and
  • 1 authors
  • 1 vignette
  • 2 internal data files
  • 14 imported packages
  • 13 exported functions (median 29 lines of code)
  • 13 non-exported functions in R (median 28 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 16 74.9
files_vignettes 3 92.4
files_tests 8 88.2
loc_R 434 43.1
loc_vignettes 161 41.5
loc_tests 30 17.7
num_vignettes 1 64.8
data_size_total 112963 84.1
data_size_median 56481 88.8
n_fns_r 26 35.7
n_fns_r_exported 13 53.9
n_fns_r_not_exported 13 28.8
n_fns_per_file_r 1 0.2 TRUE
num_params_per_fn 3 33.6
loc_per_fn_r 28 74.9
loc_per_fn_r_exp 29 61.6
loc_per_fn_r_not_exp 28 76.0
rel_whitespace_R 5 19.2
rel_whitespace_vignettes 21 25.8
rel_whitespace_tests 7 5.2
doclines_per_fn_exp 32 36.7
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 7 27.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

pkgcheck.yaml
R-CMD-check.yaml

GitHub Workflow Results

id name conclusion sha run_number date
7901669279 pkgcheck success e76e3d 22 2024-02-14
7901669272 R-CMD-check success e76e3d 10 2024-02-14

3b. goodpractice results

R CMD check with rcmdcheck

R CMD check generated the following notes:

  1. checking for hidden files and directories ... NOTE
    Found the following hidden files and directories:
    .github
    These were most likely included in error. See section ‘Package
    structure’ in the ‘Writing R Extensions’ manual.
  2. checking top-level files ... NOTE
    File
    LICENSE
    is not mentioned in the DESCRIPTION file.
  3. checking package subdirectories ... NOTE
    Found the following CITATION file in a non-standard place:
    CITATION.cff
    Most likely ‘inst/CITATION’ should be used instead.
  4. checking R code for possible problems ... NOTE
    clique_community_names: no visible binding for global variable ‘com’
    clique_community_names: no visible binding for global variable
    ‘com_name’
    clique_community_names_par: no visible binding for global variable ‘i’
    clique_community_names_par: no visible binding for global variable
    ‘com’
    clique_community_names_par: no visible binding for global variable
    ‘com_name’
    dendro_network: no visible binding for global variable ‘r’
    dendro_network: no visible binding for global variable ‘sgc’
    dendro_network: no visible binding for global variable ‘p’
    dendro_network: no visible binding for global variable ‘series_a’
    dendro_network: no visible binding for global variable ‘series_b’
    find_all_cpm_com: no visible binding for global variable ‘node’
    find_all_cpm_com: no visible binding for global variable ‘com_name’
    find_all_cpm_com: no visible binding for global variable ‘n’
    gn_names: no visible binding for global variable ‘GN_com’
    gn_names: no visible binding for global variable ‘node’
    gn_names: no visible binding for global variable ‘com_name’
    Undefined global functions or variables:
    com com_name GN_com i n node p r series_a series_b sgc

R CMD check generated the following check_fails:

  1. rcmdcheck_stale_license_file
  2. rcmdcheck_citation_file_at_standard_place
  3. rcmdcheck_undefined_globals
  4. rcmdcheck_hidden_files_and_directories

Test coverage with covr

Package coverage: 91.44

Cyclocomplexity with cyclocomp

The following function have cyclocomplexity >= 15:

function cyclocomplexity
cyto_clean_styles 20

Static code analyses with lintr

lintr found the following 96 potential issues:

message number of times
Avoid 1:length(...) expressions, use seq_len. 2
Avoid library() and require() calls in packages 2
Lines should not be more than 80 characters. 92


4. Other Checks

Details of other checks (click to open)

✖️ The following function name is duplicated in other packages:

    • sim_table from BCEA


Package Versions

package version
pkgstats 0.1.3.11
pkgcheck 0.1.2.15


Editor-in-Chief Instructions:

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

@ldecicco-USGS
Copy link

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

@ropensci-review-bot
Copy link
Collaborator

Assigned! @ldecicco-USGS is now the editor

@ldecicco-USGS
Copy link

ldecicco-USGS commented Feb 19, 2024

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

I was able to run the example functions from the readme and vignette. It would very helpful to push a compiled pkgdown site to a GitHub Page. You can set that up with:

usethis::use_pkgdown_github_pages()

Setting up a pkgdown site should take care of most of the "Documentation " subtasks. The other sub-category that could be improved is "not just perfunctory" vignettes. Perhaps you could take the contents of the "paper.md" and put that same information in a vignette?

I would also ask that you take a look at the following packages and make sure there's not specific overlap:

https://github.com/AndyBunn/dplR
https://github.com/hanecakr/fellingdateR
https://github.com/ropensci/MtreeRing

There's a good chance I'd be asking authors from those packages to do a review. I do not know enough about tree ring science to understand where the overlap might fall.


@RonaldVisser
Copy link
Author

RonaldVisser commented Feb 20, 2024

Thank you for your helpful comments! More steps to make my package better :)

The paper is there to submit to JOSS in a later stage, but I have added an extra Vignette and I'll think about developing more Vignettes.

I have created a pkgdown website: https://ronaldvisser.github.io/dendroNetwork/ and that was and interesting and fun thing to do :)

There is no overlap with the packages you mentioned:

I believe that the dendroNetwork package can work very well with these packages, but it adds new functionality to the available R-packages and I have tried to explain that in the paper (for JOSS) I have added in the repository.
Oh, for your information, there are more packages in R for dendrochronology and I have compiled a list in the past: https://ronaldvisser.github.io/Dendro_R/

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @kaijagahm as reviewer

@ropensci-review-bot
Copy link
Collaborator

@kaijagahm added to the reviewers list. Review due date is 2024-03-14. Thanks @kaijagahm 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

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

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @gzach93 as reviewer

@ropensci-review-bot
Copy link
Collaborator

@gzach93 added to the reviewers list. Review due date is 2024-03-28. Thanks @gzach93 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

📆 @gzach93 you have 2 days left before the due date for your review (2024-03-28).

@RonaldVisser
Copy link
Author

RonaldVisser commented Mar 29, 2024

Response review Kaija Gahm

Thank you Kaija for the wonderfully detailed review. Below you will find the response to the review by kaijagahm.

Documentation

  • Installation instructions

    • Thanks for pointing out this. I have added installation instructions for Cytoscape to the readme, see commit.
  • Vignettes

    • I have updated the Readme by changing the installation instructions in the readme to devtools::install_github("RonaldVisser/dendroNetwork", build_vignettes = TRUE)

    • In addition the vignettes were updated for more clarification and renamed some of them

    • See: commit

  • Examples

    • Thank you for finding that in wuchswerte() . That was a relict of programming. I have updated the function, see commit.

    • Thanks for noticing that t_value() was not outputting to console. This is corrected, see commit.

    • I have checked why cyto_create_gn_style(g_hol, gn_coms = g_hol_gn) fails. The example was not complete. The command cyto_create_graph(g_hol) was missing. In addition there was a very stupid typo in the function, because DendroNetwork was called instead of dendroNetwork. See commit which also includes a similar update for cyto_create_cpm_style() . This solves both issues

  • Community guidelines:

    • Thanks! URLBugReports and Maintainer were already in the DESCRIPTION file, but a CONTRIBUTION.md file was missing. This is added, see commit

    • The NEWS file has been replaced by a NEWS.md file, see commit

Functionality

  • Automated tests: I have also never tested an installed packaged. Here is the output of my tests:
ℹ Testing dendroNetwork
✔ | F W  S  OK | Context 
✔ |          1 | clique_community_names 
✔ |          1 | clique_community_names_par                                       
✔ |          2 | dendro_network                                                 
✔ |          2 | find_all_cpm_com                                                 
✔ |          2 | gn_names                                                         
✔ |          2 | sim_table                                                        
✔ |          1 | wuchwerte                                                        
══ Results ════════════════════════════════════════════════════════════════════════
Duration: 6.2 s 
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 11 ]
  • Packaging guidelines

    • Package API: thanks for your comments, also on the naming. I like your idea of having all functions starting with dendro_ , but I feel that would make the names of the functions too long, and some of them are already long. I have gone through all the names, but I can not find reasons to change the names of the functions, nor can I think of better names for the functions.

    • Input checking:

      • Thanks for pointing this out! All functions that need an igraph object are now checked and a check is added to sim_table, see commit.
  • You were added as reviewer to the DESCRIPTION-file, see commit

Review Comments

  1. Thanks for pointing this out. The packaged is aimed at dendrochronologists who would generally know what an rwl object is, or what a sitechronology is. I have added some explanation of the intended audience and guiding users to more information.

  2. I have added some more explanation to the help of the function sim_table() . I thought explained this sufficiently, but after reading your comments you have shown me that I was wrong. I really hope that it is more clear now.

  3. I think that this should be obvious to people in the domain. The naming of each series in a rwl data.frame generally follows a certain pattern (with deviations): Location_id, sample number and measurement (radius) number. ABC0011 would be location ABC, sample 001 and you can take several measurements along various radii in a tree trunk, in this case 1. You can make networks of measurements, but you can also group several radii into one node in the network. I hope that I am clear and that this explains it sufficiently?

  4. Thanks! I have checked and gone through everything and I hope that I have made this clear.

  5. Good point! I have created a separate vignette for this and explained that people should always use clique_community_names() initially and clique_community_names_par() only when the former is too slow. I have also update the readme to make this more clear.

  6. Oh thank you! Somewhere or sometime while writing the function I seem to have made a mistake. It is now working properly again!

The following commit addresses the points in 1-6.

I am really happy with all the things you have found and this resulted a new version to release: 0.5.2

@gzach93
Copy link

gzach93 commented Apr 4, 2024

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

I think the author did a great job connecting their R package to the already existing R packages specifically created for their field.

  • Installation instructions: for the development version of package and any non-standard dependencies in README

The installation instructions were easy to follow and I was able to install the package and realted software needed.

It seems like the audience for this package would be new researchers to dendrology that are looking to create dendrochonological networks. If this is the intended audience maybe the author can add a little more about what Cytoscape is and does or provide a small summary and where the reader can find more resources? I want to make sure that I understand but is Cytoscape mainly used for the visualization after this R packages formats the data and run the network analyses?

  • Vignette(s): demonstrating major functionality that runs successfully locally

Could you add the data description for the example data in the vignettes? I see it in RING_Visser_2021.Rd, but it might be good to move that into the vignette.

Can more specific details about what the code is doing be added? Consider also adding a brief description of things like the Girvan-Newman algorithm or more technical terms. Again I am assuming that this package is trying to create an easy pathway for researchers getting into this sort of analysis.

  • Function Documentation: for all exported functions
  • Examples: (that run successfully locally) for all exported functions

The t_value function could use a return function at the end of it. The example of the t_value function does not output results right now.

The wuchswerte function example, I think, require a dataset "anos1" from the dplR package. However, in order to use this dataset users have to visit https://opendendro.github.io/dplR-workshop/ to get started. Is it possible to make this data more accessible or use another dataset?

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

I am not sure how to run these tests.

  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 4

  • 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

  1. From my read through, I assumed that the audience was a new dendrologist of someone less familiar with creating these analyzes and this package creates an easier enterance point to help them. It think that this package does pretty good with this. However, maybe the vingettes can be expanded a little, offering another example dataset to work with and added definition for more field related jargon.

  2. In the vignettes can you add a description of the example data. It might help new users better connect their datasets to the example data and think about the correct formatting needed.

  3. I am not a dendrologist and I am not familiar with Cytoscape, so I am not sure if this is a familiar enough program where users do not need more information about what this program is used for or not. But, I think it might be useful to add more information about what Cytoscape is and some of the uses it has.

@ldecicco-USGS
Copy link

@ropensci-review-bot submit review #627 (comment) time 4

@ropensci-review-bot
Copy link
Collaborator

Logged review for gzach93 (hours: 4)

@ldecicco-USGS
Copy link

Thanks @gzach93! One way to run the tests (these are the "testthat" tests in the "test" folder) is to run devtools::test(). There are a few other options (you could go into the test folder and run them line-by-line even). The comment "Unit tests cover essential functions of the package and a reasonable range of inputs and conditions." means you are looking at the set of tests that the author includes to make sure they are testing a good-enough set of conditions on their functions. From the automated ropensci-bot, we know "Package coverage is 91.4%.". That's pretty good! It means 91.4% of the code is somehow tested in the testthat folder. The reviewer's job is just to make sure those tests seem adequate (which is subjective, but you can generally get a pretty good idea if the author's doing their due-diligence). If you knew of some important edge-cases for example, you would maybe want the developers to be testing those conditions.

@RonaldVisser you can now start addressing the reviews!

@RonaldVisser
Copy link
Author

RonaldVisser commented Apr 5, 2024

Response to review Zachary Gajewski (gzach93)

Thank you for your review, @gzach93. I am happy that you were positive about many things, but also for finding things to improve the package! Some things were already corrected before receiving your review and I am sorry for already updating while waiting for your review.

Documentation

Installation instructions: Thanks for noticing that. I have added a description of Cytoscape to the README and the vignettes: dendroNetwork and Cytoscape, including a link to the Cytoscape-website., see commit.

Vignettes:

I have added a vignette on dendrochronological data, see commit.

Links to a better explanation of the Girvan-Newman Algorithm and CPM were already added earlier, based on the review by @kaijagahm, see response and commit

Examples:

Thanks for also noticing that t_value() was not outputting to console. This was already corrected, based on the other review: see commit.

The references to the anos1 dataset was also already corrected, see commit

 Automated tests: see response to the review by kaijagahm

You were added as reviewer to the DESCRIPTION-file, see commit

Review Comments

  1. Thanks for pointing this out. I have update various things in the readme and the vignettes. I hope that this is now more clear
  2. I have added a vignette on the data, so this should help.
  3. I have added some description for Cytoscape and also some links to find more help.

I would like to explain that a dendrology is not the same as dendrochronology. Both are tree related, but the latter is aimed at tree-rings and the temporal patterns and the former more on trees in general. This confusion is very common, since these words look very similar.

Thank you again for your help in making this package and the documentation better!

@RonaldVisser
Copy link
Author

RonaldVisser commented Apr 8, 2024

@ldecicco-USGS : I have addressed both reviews.

I am really happy with the improvements based on the valuable comments by reviewers @kaijagahm and @gzach93, and I hope I have addressed the points they raised sufficiently.

@ldecicco-USGS
Copy link

Hi @gzach93 and @kaijagahm

Do you feel that the changes and updates have addressed your concerns?

If so, see:
https://devdevguide.netlify.app/approval2template

@ropensci-review-bot
Copy link
Collaborator

@RonaldVisser: 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

@gzach93
Copy link

gzach93 commented Apr 23, 2024

Hi @RonaldVisser thank you for going through and addressing all my comments!

Reviewer Response #### Final approval (post-review) - [X] The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

@ldecicco-USGS
Copy link

Thanks again @gzach93 and @kaijagahm for the reviews and @RonaldVisser for the submission!

@ldecicco-USGS
Copy link

@ropensci-review-bot approve dendroNetwork

@ropensci-review-bot
Copy link
Collaborator

Approved! Thanks @RonaldVisser for submitting and @kaijagahm, @gzach93 for your reviews! 😁

To-dos:

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

Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent).

Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @ropensci/blog-editors in your reply. They will get in touch about timing and can answer any questions.

We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved.

Last but not least, you can volunteer as a reviewer via filling a short form.

@kaijagahm
Copy link

Reviewer Response

Final approval (post-review)

  • [ X] The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 4.5 (total, not just for this step)

@RonaldVisser
Copy link
Author

@ropensci-review-bot finalize transfer of dendroNetwork

@ropensci-review-bot
Copy link
Collaborator

Transfer completed.
The dendroNetwork team is now owner of the repository and the author has been invited to the team

@RonaldVisser
Copy link
Author

Thanks @ldecicco-USGS for your excellent role as editor! And thanks again @kaijagahm and @gzach93 for your wonderful reviews.

The package is now transferred to ROpenSci and I have addressed all to do's

I'd be happy to write a blog post about this packages @ropensci/blog-editors

@steffilazerte
Copy link
Member

Hi @RonaldVisser!

I'm excited to hear that you're interested in writing a blog post for us!

As mentioned above, we have two types of blog posts: either an introduction to your package featuring technical details for a technical audience or a longer post with some narrative about its development or something you learned, and possibly an example of its use for a broader readership. The difference between a technote and a blog post is more about the style and content then length, but technical posts are often shorter than blog posts (no requirement either way).

To contribute a post, we'll have you open a PR to the roweb3 repository. About a week or two before our publication date, you can ping me and I'll review the post and give you some suggestions for content and style. You'll consider my feedback, make any changes you'd like to make, and then we merge and publish! We have a flexible timeline right now, so when every you're ready is good for us. However, if you work better with a deadline, we could set a deadline of the last week of May for the draft, expecting to publish the first week of June (or earlier!).

You can read more about the details of writing an rOpenSci blog post here: https://blogguide.ropensci.org/ (and don't hesitate to ask if you have any questions).

So! Let me know what kind of post you'd like to write and what kind of timeline you want to follow. You can reach me on the Slack (Steffi LaZerte; you should have gotten an invitation by now?) or via email (sel@steffilazerte.ca), and/or ping me when you open the PR to roweb3 (@steffilazerte).

Thanks!

@RonaldVisser
Copy link
Author

Hi @steffilazerte ,

Thanks for the comments! I'll have a more through look at the documentation and I'll probably email you with questions later. I have not received a Slack-invite, but I have not used that for some time...

Thank you :)

@steffilazerte
Copy link
Member

Great! Well you should get the Slack invite at some point (I think both people who handle that are away at the moment). And if you don't (and would like it 😁 ), let us know!

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

7 participants