software-review fellingdateR: Estimate, report and combine felling dates of historical tree-ring series

fellingdateR: Estimate, report and combine felling dates of historical tree-ring series

Open hanecakr opened this issue 9 months ago • 45 comments

Submitting Author Name: Kristof Haneca Submitting Author Github Handle: @hanecakr Other Package Authors Github handles: (comma separated, delete if none) Repository: https://github.com/hanecakr/fellingdateR Version submitted: Submission type: Standard Editor: @maelle Reviewers: @njtierney, @ajpelu

Archive: TBD Version accepted: TBD Language: en

Paste the full DESCRIPTION file inside a code block below:

Package: fellingdateR
Title: Estimate, report and combine felling dates of historical tree-ring 
    series
Version: 0.0.0.9003
Authors@R: c(
    person("Kristof", "Haneca", , "[email protected]",role = c("aut", "cre"),
        comment = c(ORCID = "0000-0002-7719-8305")),
    person("Koen", "Van Daele", role = "ctb",
        comment = c(ORCID = "0000-0002-8153-2978")),
    person("Ronald", "Visser", role = "ctb",
        comment = c(ORCID = "0000-0001-6966-1729"))    
    )
Description: `fellingdateR` is an R package that aims to facilitate the 
  analysis and interpretation of tree-ring data from wooden 
  cultural heritage objects and structures. The package standarizes the process
  of computing and combining felling date estimates, both for individual and 
  groups of related tree-ring series.
URL: https://github.com/hanecakr/fellingdateR
BugReports: https://github.com/hanecakr/fellingdateR/issues
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Depends: 
    R (>= 2.10)
LazyData: true
Imports: 
    dplyr,
    ggplot2,
    ggtext,
    MASS,
    plyr,
    tidyr,
    utils,
    dplR
Suggests: 
    covr,
    knitr,
    rmarkdown,
    testthat (>= 3.0.0)
Config/testthat/edition: 3
VignetteBuilder: knitr

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
- [x] 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 aims to facilitate and standardize the computation of felling dates from dated tree-ring series. It covers all steps of processing measured ring-width series from raw data to the reporting of felling date estimates, for single series and for groups of related tree-ring series.

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

Maily professional tree-ring scientists (dendrochronologists) that work on historical timbers and wooden objects (archaeology, architectural history, sculptures, panel paintings, etc.)

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, no other R-packages do the same job.

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

yes

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.

All checks pass

Technical checks

Confirm each of the following by checking the box.

[x ] I have read the rOpenSci packaging guide.
[x ] I have read the author guide and I expect to maintain this package for at least 2 years or to find a replacement.

This package:

[x] does not violate the Terms of Service of any service it interacts with.
[x] has a CRAN and OSI accepted license.
[x] contains a README with instructions for installing the development version.
[x] includes documentation with examples for all functions, created with roxygen2.
[x] contains a vignette with examples of its essential functions and uses.
[x] has a test suite.
[x] has continuous integration, including reporting of test coverage.

Publication options

[x] 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

[x] I agree to abide by rOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

Nov 20 '23 16:11 hanecakr

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

Nov 20 '23 16:11 ropensci-review-bot

:rocket:

The following problem was found in your submission template:

URL = [https] is not valid The package could not be checked because of problems with the URL. Editors: Please ensure these problems are rectified, and then call @ropensci-review-bot check package.

:wave:

Nov 20 '23 16:11 ropensci-review-bot

@hanecakr The error was because your URL field was markdown-style: "[url](url)", and should just be plain "url". I've edited to fix; you should now be able to call @ropensci-review-bot check package yourself to restart checks. Sorry for any inconvencience.

Nov 21 '23 08:11 mpadge

@ropensci-review-bot check package

Nov 21 '23 08:11 hanecakr

Thanks, about to send the query.

Nov 21 '23 08:11 ropensci-review-bot

:rocket:

Editor check started

:wave:

Nov 21 '23 08:11 ropensci-review-bot

Checks for fellingdateR (v0.0.0.9003)

git hash: 05c449fb

:heavy_check_mark: Package name is available
:heavy_check_mark: has a 'codemeta.json' file.
:heavy_check_mark: has a 'contributing' file.
:heavy_check_mark: uses 'roxygen2'.
:heavy_check_mark: 'DESCRIPTION' has a URL field.
:heavy_check_mark: 'DESCRIPTION' has a BugReports field.
:heavy_check_mark: Package has at least one HTML vignette
:heavy_check_mark: All functions have examples.
:heavy_check_mark: Package has continuous integration checks.
:heavy_check_mark: Package coverage is 77.6%.
:heavy_check_mark: R CMD check found no errors.
:heavy_check_mark: R CMD check found no warnings.
:eyes: Function names are duplicated in other packages

(Checks marked with :eyes: may be optionally addressed.)

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	base	363
internal	stats	33
internal	fellingdateR	26
internal	grDevices	7
internal	graphics	2
imports	ggplot2	32
imports	utils	15
imports	dplyr	6
imports	ggtext	3
imports	plyr	2
imports	tidyr	2
imports	dplR	2
imports	MASS	1
suggests	covr	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

sub (32), rep (31), length (21), c (17), nrow (17), apply (15), matrix (15), max (14), ncol (13), data.frame (10), seq (10), which (10), range (9), for (8), paste0 (8), as.data.frame (7), attributes (7), numeric (7), as.numeric (6), by (6), drop (5), is.na (5), mean (5), min (5), list (4), summary (4), diff (3), floor (3), format (3), rownames (3), setdiff (3), sum (3), T (3), unique (3), any (2), character (2), get (2), grep (2), lapply (2), names (2), round (2), sapply (2), scale (2), seq_len (2), abs (1), all (1), as.character (1), as.integer (1), ceiling (1), colnames (1), colSums (1), complex (1), cumsum (1), dim (1), file.exists (1), gettext (1), Im (1), intersect (1), lengths (1), logical (1), match (1), merge (1), message (1), Re (1), readLines (1), regexpr (1), seq_along (1), sort (1), sqrt (1), substring (1), table (1), vapply (1)

stats

df (18), spline (7), end (3), sd (2), start (2), sigma (1)

ggplot2

element_blank (13), element_text (5), ggplot (3), aes (2), arrow (2), geom_line (2), unit (2), geom_area (1), scale_x_continuous (1), theme (1)

fellingdateR

hdi (7), sw_model (6), sw_data_overview (3), d.count (2), d.dens (2), fd_report (2), sw_interval (2), strip.comment (1), sw_combine_plot (1)

utils

data (13), citation (1), tail (1)

grDevices

pdf (7)

dplyr

left_join (1), mutate (1), pull (1), relocate (1), select (1), summarize (1)

ggtext

element_markdown (3)

dplR

rwl.stats (2)

graphics

title (2)

plyr

round_any (2)

tidyr

pivot_longer (2)

MASS

fitdistr (1)

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
13 internal data files
8 imported packages
15 exported functions (median 80 lines of code)
21 non-exported functions in R (median 50 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	1	68.4
files_tests	14	93.8
loc_R	2855	89.8
loc_vignettes	193	48.1
loc_tests	480	74.1
num_vignettes	1	64.8
data_size_total	6023	68.2
data_size_median	465	60.0
n_fns_r	36	45.9
n_fns_r_exported	15	58.5
n_fns_r_not_exported	21	42.1
n_fns_per_file_r	1	20.7
num_params_per_fn	4	67.3
loc_per_fn_r	65	93.5
loc_per_fn_r_exp	80	89.2
loc_per_fn_r_not_exp	50	90.3
rel_whitespace_R	11	80.9
rel_whitespace_vignettes	68	73.3
rel_whitespace_tests	11	58.1
doclines_per_fn_exp	36	43.7
doclines_per_fn_not_exp	0	0.0	TRUE
fn_call_network_size	30	55.5

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

GitHub Workflow Results

id	name	conclusion	sha	run_number	date
6932735657	pkgcheck	success	05c449	5	2023-11-20
6932735666	R-CMD-check	success	05c449	9	2023-11-20
6932735656	test-coverage	success	05c449	12	2023-11-20

3b. `goodpractice` results

`R CMD check` with rcmdcheck

R CMD check generated the following note:

checking data for non-ASCII characters ... NOTE Note: found 11 marked UTF-8 strings

R CMD check generated the following check_fails:

cyclocomp
rcmdcheck_non_ascii_characters_in_data

Test coverage with covr

Package coverage: 77.6

Cyclocomplexity with cyclocomp

The following functions have cyclocomplexity >= 15:

function	cyclocomplexity
read_fh	150
fd_report	42
sw_sum	32
cor_table	31
sw_combine	27
sw_interval	17
movAv	16
sw_combine_plot	15

Static code analyses with lintr

lintr found the following 170 potential issues:

message	number of times
Avoid 1:length(...) expressions, use seq_len.	6
Avoid library() and require() calls in packages	1
Avoid using sapply, consider vapply instead, that's type safe	2
Lines should not be more than 80 characters.	154
Use <-, not =, for assignment.	7

4. Other Checks

Details of other checks (click to open)

:heavy_multiplication_x: The following 3 function names are duplicated in other packages:

- get_header from eventr
- hdi from bayestestR, ggdist, hdi, HDInterval
- movAv from berryFunctions

Package Versions

package	version
pkgstats	0.1.3.9
pkgcheck	0.1.2.11

Editor-in-Chief Instructions:

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

Nov 21 '23 09:11 ropensci-review-bot

@hanecakr Thank you for the submission. All looks good to me. Will pass on the handling editor momentarily.

Nov 23 '23 15:11 jhollist

@ropensci-review-bot assign @maelle as editor

Nov 23 '23 15:11 jhollist

Assigned! @maelle is now the editor

Nov 23 '23 15:11 ropensci-review-bot

Thanks for your submission @hanecakr! I have some comments before I can proceed with the reviewer search. :deciduous_tree:

Happy to discuss the comments below! :smile_cat:

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,
- [x] 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?
[x] 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?
[x] Contributing information: Is the documentation for contribution clear enough e.g. tokens for tests, playgrounds?
[x] License: The package has a CRAN or OSI accepted license.
[x] 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

Installation instructions

For brevity you can keep only the installation instructions with pak.

Dependencies

I wonder whether the dependency on plyr for very few functions could be avoided?

Metadata

I don't think it's allowed to have backticks in a DESCRIPTION file, I see some in the Description field. Or if it's allowed, at least it's unusual?

Docs

I'd recommend creating a pkgdown website. In the vignette and README it'd make sense to remove the description of functions and datasets, instead pointing to the reference index of the pkgdown website. It'd make it easier to skim the README, and to see what functions/datasets there are at once (for reviewers but also users!).

Data

I see the example datasets are saved as internal datasets however you use them in the documentation (in the README) so why not make them exported datasets? https://r-pkgs.org/data.html#sec-data-data

Rather than using the names with "dummy" I'd recommend informative names. (Beside, some might consider "dummy" to be an offensive word related to ableism).

#### Tests

I see some top-level code in for instance https://github.com/hanecakr/fellingdateR/blob/master/tests/testthat/test-fd_report.R I'd recommend instead having a function defined in a helper file like tests/testthat/helper-testdata

test_data <- function() {
data.frame(series = c("aaa", "bbb", "ccc", "no_last", "no_sapwood"),
                       n_sapwood = c(10, 11, 12, 10, NA),
                       waneyedge = c(FALSE, FALSE, TRUE, FALSE, FALSE),
                       last = c(123, 456, 1789, NA, 1978))
}

that you'd then call from each test (not each test file) to create the data. Even better, it could have a more informative name. For more context: https://blog.r-hub.io/2020/11/18/testthat-utility-belt/#code-called-in-your-tests and https://r-pkgs.org/testing-advanced.html#sec-testing-advanced-fixture-helper With such a strategy, each test is easier to read and to run, as it's self-contained.

I think some test lines could be more condensed, e.g. https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/tests/testthat/test-fd_report.R#L105 could go on the previous line, and https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/tests/testthat/test-fd_report.R#L96-L98 or https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/tests/testthat/test-fd_report.R#L120-L122 might be a single line, to see more of the test file at once on the screen?

You shouldn't namespace fellingdateR::: data in test files.

For test-sw_combine_plot.R you might want to look into snapshot testing rather than using ggplot2's internal data structure: https://testthat.r-lib.org/articles/snapshotting.html#whole-file-snapshotting (other relevant source: https://www.tidyverse.org/blog/2022/09/playing-on-the-same-team-as-your-dependecy/#testing-testing)

Why is there a test that is commented out? https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/tests/testthat/test-sw_interval.R#L2 (as opposed to, say, completely removed)

Instead of the regular expressions for testing errors, you might be interested in this approach of classed errors: https://www.mm218.dev/posts/2023-11-07-classed-errors/

Code

Please read the output from lintr in the automatic checks above, and fix accordingly (or update this thread to explain why you disagree).

Lines such as https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/R/cor_table.R#L117 should use inherits()

if (!inherits(x, "rwl")) {

In for instance cor_table.R, I think the code might be more readable with explaining variables. For instance

if (!all(diff(as.numeric(row.names(x))) == 1)) {

would be

increasing_consecutive_years <- (all(diff(as.numeric(row.names(x))) == 1))
if (!increasing_consecutive_years) {

Same comments as for test files, I think there could be fewer new lines in scripts such as https://github.com/hanecakr/fellingdateR/blob/master/R/cor_table.R, to facilitate reading / vertical scrolling. I guess this conflicts a bit with the huge indentations in the files (which you are obviously allowed to prefer!).

For the else if in https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/R/sw_interval.R#L236, why not use the switch() function? I must confess I like using it but have to look up the docs every time to remember how to input the arguments. But it'll decrease the complexity of the current logic.

In https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/R/sw_interval_plot.R#L137 what warnings are suppressed? In the same post mentioned previously https://www.mm218.dev/posts/2023-11-07-classed-errors/, it's shown that one can selectively suppress warnings of a given class.

Nov 27 '23 11:11 maelle

Thanks a lot @maelle for this first round of comments. Will try to tackle most of these issues in the next few days.

Nov 28 '23 12:11 hanecakr

Dear @maelle. I've been working on the package and think I'm able now to address most of your comments. Thanks a lot for your time, valuable insights and recommendations!

I'll list the answers to your comments here:

Installation instructions

For brevity you can keep only the installation instructions with pak.

[x ] Ok. I've removed the install with devtools from README

Dependencies

I wonder whether the dependency on plyr for very few functions could be avoided?

[x ] The function plyr::round_any is now removed from sw_interval_plot(), sw_combine_plot() and sw_sum_plot()
[x ] floor/ceiling(x/10)*10 now does the job

Metadata

I don't think it's allowed to have backticks in a DESCRIPTION file, I see some in the Description field. Or if it's allowed, at least it's unusual?

[x ] Backticks removed from DESCRIPTION

Docs

I'd recommend creating a pkgdown website.

[x ] Initiated a pkgdown website now. I had postponed this because I also have a short paper ready (after code-review) for JOSS. Planned to use that paper (and reviewer comments) for an article/vignette on a pkgdown site. But obviously, for reviewers a pkgdown-site is most helpful at this stage already.

Data

I see the example datasets are saved as internal datasets however you use them in the documentation (in the README) so why not make them exported datasets? https://r-pkgs.org/data.html#sec-data-data

[x ] The exported data are factual (published) data that are used by nearly all functions in the package. With sw_data_overview() users get an overview of what sapwood datasets are available in the package and with sw_data_info() basic descriptives of those dataset.
[ ] When I would make the made-up date also external, that would mix real-world data and made-up data. And would also complicate the code for sw_data_overview(). Therefore I would like to keep the made-up data for examples and tests as internal data.

Rather than using the names with "dummy" I'd recommend informative names. (Beside, some might consider "dummy" to be an offensive word related to ableism).

[x ] Thanks for pointing this out. Changed this throughout all functions and tests to “trs_example1” (tree-ring series example). Indeed much better than ‘dummy’ or ‘fake’.

Tests

[x ] The top-level code is in fact redundant. The test can equally be run with the internal data. This has now been implemented in the tests.

I think some test lines could be more condensed, e.g. ...

[x ] I reformatted most test-files to make them more condensed. Should have done that before submission…

You shouldn't namespace fellingdateR::: data in test files.

[x ] Right. Removed the namespace to fellingdateR in tests.

For test-sw_combine_plot.R you might want to look into snapshot testing rather than using ggplot2's internal data structure: https://testthat.r-lib.org/articles/snapshotting.html#whole-file-snapshotting (other relevant source: https://www.tidyverse.org/blog/2022/09/playing-on-the-same-team-as-your-dependecy/#testing-testing)

[ ] I've left this as it is now. Can't find an elegant way to implement snapshot testing right now. Need to look deeper into that.

[x ] Oops, Removed this from the test-file

Instead of the regular expressions for testing errors, you might be interested in this approach of classed errors: https://www.mm218.dev/posts/2023-11-07-classed-errors/

Code

Please read the output from lintr in the automatic checks above, and fix accordingly (or update this thread to explain why you disagree).

Lines such as https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/R/cor_table.R#L117 should use inherits() if (!inherits(x, "rwl")) {

[x ] Now using inherits() in the latest version of cor_table().

In for instance cor_table.R, I think the code might be more readable with explaining variables. For instance if (!all(diff(as.numeric(row.names(x))) == 1)) { would be increasing_consecutive_years <- (all(diff(as.numeric(row.names(x))) == 1)) if (!increasing_consecutive_years) {

[x ] OK, I've changed this to increase readability

[x ] I’ve reformatted the code with RStudio > reformat code. Is indeed (a bit) more condensed now.

For the else if in https://github.com/hanecakr/fellingdateR/blob/05c449fb13b678ac1700177d73a20fad6c55da8c/R/sw_interval.R#L236, why not use the switch() function? I must confess I like using it but have to look up the docs every time to remember how to input the arguments. But it'll decrease the complexity of the current logic.

[ ] I remember I tried this before, but failed to implement switch() for this function. Would prefer to leave it as it is now.

[x ] Doesn’t seem to be necessary anymore to use suppressWarnings() to avoid messages from ggplot, for sw_model() and sw_combine(). So I’ve removed the calls to suppressWarnings() for these functions.
[ ] For sw_interval() however I’ve kept the call to suppressWarnings to avoid warnings as:

 Warning: Removed 52 rows containing non-finite values (stat_align()).  Warning: Removed 43 rows containing missing values (geom_line()).

I'm sure there are more elegant and effective ways to improve the scripts, functions and their documentation, but I hope that these initial revisions have elevated the quality of the package sufficiently, making it amenable for the review process. Of course, I'm open for additional insights and recommendations to further improved the quality and performance of the package.

-- Kristof

Nov 28 '23 19:11 hanecakr

Thanks a ton @hanecakr!

A small note, in GitHub Markdown if you type - [x] the checkbox appears as a ticked checkbox, whereas - [x ] does not.

In the README when mentioning the reference index, you could rephrase the link so as not to use "here" (https://webaccess.berkeley.edu/ask-pecan/click-here), for instance "You can find the [list of functions]".

When I would make the made-up date also external, that would mix real-world data and made-up data. And would also complicate the code for sw_data_overview(). Therefore I would like to keep the made-up data for examples and tests as internal data.

Ok, but in that case would it work to not load sysdata (which might look confusing) but instead use data(dataname, package = "fellingdateR")? Also to avoid using ::: when using the data in examples.

Should have done that before submission…

There's already a lot of stuff to do before submission!

Don't hesitate to ask if you need help with snapshot testing, but it's maybe not useful here anyway.

A small non compulsory thing, I see your default branch is named master, you could change it to the new community standard "main" using the usethis function for instance. See also https://www.tidyverse.org/blog/2021/10/renaming-default-branch/

Another Git thing, your repository contains the .Rproj.user folder, should it be removed and added to .gitignore? See https://usethis.r-lib.org/reference/git_vaccinate.html for a handy way to gitignore it globally.

I remember I tried this before, but failed to implement switch() for this function. Would prefer to leave it as it is now.

Fair enough! Happy to try and make a PR if you change your mind (maybe this time I'd remember how to use it straight away :joy: )

I'll now look for reviewers! Thanks again for all your work!

Nov 30 '23 07:11 maelle

@ropensci-review-bot seeking reviewers

Nov 30 '23 07:11 maelle

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

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

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

Nov 30 '23 07:11 ropensci-review-bot

@maelle Thanks a lot! Meanwhile, I will change the branch name to main, use the data(...) function to load internal data, and have a look at the .Rproj.user folder and try to find a way to add it to .gitignore. The badge will be added to README soon.

Nov 30 '23 16:11 hanecakr

@ropensci-review-bot add @njtierney to reviewers

Dec 01 '23 05:12 maelle

@njtierney added to the reviewers list. Review due date is 2023-12-22. Thanks @njtierney 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.

Dec 01 '23 05:12 ropensci-review-bot

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

Dec 01 '23 05:12 ropensci-review-bot

Thank you @njtierney for accepting to review this package!

Dec 01 '23 05:12 maelle

@njtierney: with a little delay, I decided to follow the advice of @maelle and moved the example datasets from internal to external data, so they become easily available for testing and examples, both for end-uses as during code review. The latest commit implements the necessary (small) changes. Looking forward to your comments and suggestions. -- Kristof.

Dec 01 '23 15:12 hanecakr

Thanks, team! I've got a few deadlines this week but will take a look next week and aim to have this done by the 22nd.

Dec 04 '23 22:12 njtierney

Thank you @njtierney and good luck with the deadlines!

Dec 05 '23 08:12 maelle

@ropensci-review-bot add @ajpelu to reviewers

Dec 09 '23 08:12 maelle

@ajpelu added to the reviewers list. Review due date is 2023-12-30. Thanks @ajpelu for accepting to review! Please refer to our reviewer guide.

Dec 09 '23 08:12 ropensci-review-bot

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

Dec 09 '23 08:12 ropensci-review-bot

Thank you @ajpelu for accepting to review this package!

Dec 09 '23 08:12 maelle

:calendar: @njtierney you have 2 days left before the due date for your review (2023-12-22).

Dec 20 '23 05:12 ropensci-review-bot

Hi team!

I'm still working through this review. I've done a first pass, but I am just finishing up collecting together my points and polishing up the review so it can be easy to follow. Should be done later this afternoon! :)

Dec 22 '23 06:12 njtierney

software-review software-review copied to clipboard

fellingdateR: Estimate, report and combine felling dates of historical tree-ring series

Scope

Technical checks

Publication options

Code of conduct

Checks for fellingdateR (v0.0.0.9003)

1. Package Dependencies

2. Statistical Properties

2a. Network visualisation

3. goodpractice and other checks

3a. Continuous Integration Badges

3b. goodpractice results

R CMD check with rcmdcheck

Test coverage with covr

Cyclocomplexity with cyclocomp

Static code analyses with lintr

4. Other Checks

Editor-in-Chief Instructions:

Editor checks:

Editor comments

Installation instructions

Dependencies

Metadata

Docs

Data

Code

Installation instructions

Dependencies

Metadata

Docs

Data

Tests

Code

software-review
software-review copied to clipboard

3. `goodpractice` and other checks

3b. `goodpractice` results

`R CMD check` with rcmdcheck