mapmetadata #674

RayStick · 2024-11-26T16:00:00Z

Submitting Author Name: Rachael Stickland
Submitting Author Github Handle: @RayStick
Other Package Authors Github handles: (comma separated, delete if none) @BatoolMM, @Rainiefantasy
Repository: https://github.com/aim-rsf/mapmetadata
Version submitted:
Submission type: Standard
Editor: @maelle
Reviewers: @ymansiaux, @Lextuga007

Archive: TBD
Version accepted: TBD
Language: en

Paste the full DESCRIPTION file inside a code block below:

Type: Package
Package: mapmetadata
Title: Map health metadata onto predefined research domains
Version: 3.0.0
Authors@R: c(
    person("Rachael", "Stickland", , "[email protected]", role = c("aut", "cre"),
           comment = c(ORCID = "0000-0003-3398-4272")),
    person("Batool", "Almarzouq", role = "ctb",
           comment = c(ORCID = "0000-0002-3905-2751")),
    person("Mahwish", "Mohammad", role = "ctb",
           comment = c(ORCID = "0009-0004-5295-0726")),
    person("Daniel", "Delbarre", role = "ctb",
           comment = c(ORCID = "0000-0003-4633-4252")),
    person("Nida", "Ziauddeen", role = "ctb",
           comment = c(ORCID = "0000-0002-8964-5029"))
  )
Description: Prior to gaining full access to health datasets, explore
    publicly available metadata and map metadata onto predefined research
    domains.  This package uses structural metadata files downloaded from
    the Health Data Research Gateway (https://healthdatagateway.org/en).
    In theory, any metadata file with the same structure as the files
    downloaded from this gateway can be used with this package, but the
    package has been developed and tested on metadata files from this
    gateway only.
License: GPL (>= 3)
URL: https://aim-rsf.github.io/mapmetadata/
BugReports: https://github.com/aim-rsf/mapmetadata/issues
Depends: 
    R (>= 2.10)
Imports: 
    cli,
    dplyr,
    ggplot2,
    gridExtra,
    htmlwidgets,
    plotly,
    tidyr
Suggests: 
    devtools,
    knitr,
    mockery,
    rmarkdown,
    testthat (>= 3.0.0),
    withr
VignetteBuilder: 
    knitr
Config/testthat/edition: 3
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.3.2

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):

This package is related to data access, as interacting with health metadata can help a researcher/research group decide what datasets to access for their research, and be more informed when writing their data access request. It involves data validation checks as it checks for completeness of metadata, and visualizes this.

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

Any users of health metadata, specifically for research projects that are using large population datasets, with many latent variables (research domains/concepts) and they need to investigate which variables in the datasets map onto their research domains of interest.

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

Not that I am aware of

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

Not Applicable

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.

Not Applicable

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

Technical checks

Confirm each of the following by checking the box.

I have read the rOpenSci packaging guide.
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:

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

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

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

The text was updated successfully, but these errors were encountered:

ropensci-review-bot · 2024-11-26T16:00:03Z

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

ropensci-review-bot · 2024-11-26T16:00:07Z

🚀

Editor check started

👋

ropensci-review-bot · 2024-11-26T16:12:47Z

Checks for browseMetadata (v2.0.1)

git hash: 3a779939

✔️ Package name is available
✔️ has a 'codemeta.json' file.
✔️ has a 'contributing' file.
✔️ uses 'roxygen2'.
✔️ 'DESCRIPTION' has a URL field.
✖️ 'DESCRIPTION' does not have a BugReports field.
✔️ Package has at least one HTML vignette
✖️ These functions do not have examples: [concensus_on_mismatch, copy_previous, count_empty_desc, end_plot, join_outputs, json_table_to_df, load_data, map_metadata_compare, map_metadata_convert, ref_plot, user_categorisation_loop, user_categorisation, user_prompt_list, user_prompt, valid_comparison].
✖️ Package has no continuous integration checks.
✖️ Package coverage is 53.3% (should be at least 75%).
✔️ R CMD check found no errors.
✔️ R CMD check found no warnings.
👀 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	206
internal	browseMetadata	33
internal	stats	15
internal	graphics	13
internal	utils	8
internal	tools	1
imports	cli	30
imports	dplyr	9
imports	tidyr	3
imports	gridExtra	2
imports	htmlwidgets	2
imports	plotly	2
imports	jsonlite	2
imports	ggplot2	1
suggests	knitr	NA
suggests	rmarkdown	NA
suggests	devtools	NA
suggests	testthat	NA
suggests	mockery	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

list (57), paste0 (15), data.frame (13), c (11), for (10), return (9), character (8), nrow (8), length (6), get (5), file (3), format (3), integer (3), numeric (3), row (3), Sys.time (3), all (2), apply (2), as.list (2), list.files (2), max (2), min (2), paste (2), rbind (2), readline (2), strsplit (2), subset (2), suppressWarnings (2), t (2), unique (2), unlist (2), any (1), as.integer (1), as.matrix (1), cbind (1), do.call (1), getwd (1), gsub (1), is.na (1), lapply (1), matrix (1), nchar (1), scan (1), setdiff (1), system.file (1), unname (1), which (1)

browseMetadata

json_table_to_df (4), user_categorisation (4), ref_plot (3), user_prompt_list (3), concensus_on_mismatch (2), copy_previous (2), count_empty_desc (2), end_plot (2), join_outputs (2), load_data (2), user_prompt (2), browse_metadata (1), map_metadata (1), map_metadata_compare (1), map_metadata_convert (1), user_categorisation_loop (1)

cli

cli_alert_info (17), cli_alert_danger (4), cli_alert_success (4), cli_h1 (3), cli_alert_warning (2)

stats

family (8), line (6), df (1)

graphics

title (7), legend (3), text (3)

dplyr

n (7), join_by (1), left_join (1)

utils

read.csv (7), data (1)

tidyr

complete (3)

gridExtra

grid.arrange (1), tableGrob (1)

htmlwidgets

saveWidget (2)

jsonlite

fromJSON (2)

plotly

plot_ly (2)

ggplot2

ggsave (1)

tools

file_path_sans_ext (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 11 files) and
1 authors
2 vignettes
5 internal data files
8 imported packages
17 exported functions (median 23 lines of code)
17 non-exported functions in R (median 24 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	11	60.1
files_vignettes	2	81.7
files_tests	15	92.3
loc_R	725	56.6
loc_vignettes	190	45.1
loc_tests	410	67.1
num_vignettes	2	85.5
data_size_total	24603	74.8
data_size_median	245	55.6
n_fns_r	34	43.4
n_fns_r_exported	17	62.1
n_fns_r_not_exported	17	35.4
n_fns_per_file_r	2	34.1
num_params_per_fn	3	29.3
loc_per_fn_r	24	68.0
loc_per_fn_r_exp	23	52.4
loc_per_fn_r_not_exp	24	70.5
rel_whitespace_R	19	58.9
rel_whitespace_vignettes	43	54.0
rel_whitespace_tests	25	70.1
doclines_per_fn_exp	17	9.3
doclines_per_fn_not_exp	0	0.0	TRUE
fn_call_network_size	31	54.8

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 note:

checking R code for possible problems ... NOTE
browse_metadata: no visible binding for global variable ‘Empty’
copy_previous: no visible binding for global variable ‘data_element’
count_empty_desc: no visible binding for global variable ‘empty’
end_plot: no visible binding for global variable ‘domain_code’
join_outputs: no visible binding for global variable ‘data_element’
map_metadata: no visible binding for global variable ‘note’
Undefined global functions or variables:
data_element domain_code empty Empty note

R CMD check generated the following check_fails:

description_bugreports
no_import_package_as_a_whole
rcmdcheck_undefined_globals

Test coverage with covr

Package coverage: 53.31

The following files are not completely covered by tests:

file	coverage
R/map_metadata_compare.R	0%
R/map_metadata_convert.R	0%
R/map_metadata.R	0%

Cyclocomplexity with cyclocomp

The following function have cyclocomplexity >= 15:

function	cyclocomplexity
map_metadata	18

Static code analyses with lintr

lintr found no issues with this package!

4. Other Checks

Details of other checks (click to open)

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

- browse_metadata from OECD

Package Versions

package	version
pkgstats	0.2.0.48
pkgcheck	0.1.2.77

Editor-in-Chief Instructions:

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

RayStick · 2024-11-26T16:53:20Z

Closing issue for now, as I misread some recommendations for a package to be ready. Will address these properly (the pkgcheck results) and then re-submit. Thank you!

mpadge · 2024-11-27T08:35:42Z

@RayStick No worries. When you're ready, please just open this issue again (and not a new issue), call @ropensci-review-bot check package and things will move on from there.

RayStick · 2024-11-27T08:57:10Z

Okay I will do that, thanks @mpadge!

RayStick · 2024-12-12T14:45:54Z

@ropensci-review-bot check package

ropensci-review-bot · 2024-12-12T14:45:59Z

Thanks, about to send the query.

ropensci-review-bot · 2024-12-12T14:46:02Z

🚀

Editor check started

👋

ropensci-review-bot · 2024-12-12T14:49:16Z

Checks for browseMetadata (v2.0.2)

git hash: 57b7191b

✔️ 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 89.2%.
✔️ 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	206
internal	browseMetadata	33
internal	stats	15
internal	graphics	13
internal	utils	8
internal	tools	1
imports	cli	30
imports	dplyr	9
imports	tidyr	3
imports	gridExtra	2
imports	htmlwidgets	2
imports	plotly	2
imports	jsonlite	2
imports	ggplot2	1
suggests	knitr	NA
suggests	rmarkdown	NA
suggests	devtools	NA
suggests	testthat	NA
suggests	mockery	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

list (57), paste0 (15), data.frame (13), c (11), for (10), return (9), character (8), nrow (8), length (6), get (5), file (3), format (3), integer (3), numeric (3), row (3), Sys.time (3), all (2), apply (2), as.list (2), list.files (2), max (2), min (2), paste (2), rbind (2), readline (2), strsplit (2), subset (2), suppressWarnings (2), t (2), unique (2), unlist (2), any (1), as.integer (1), as.matrix (1), cbind (1), do.call (1), getwd (1), gsub (1), is.na (1), lapply (1), matrix (1), nchar (1), scan (1), setdiff (1), system.file (1), unname (1), which (1)

browseMetadata

json_table_to_df (4), user_categorisation (4), ref_plot (3), user_prompt_list (3), concensus_on_mismatch (2), copy_previous (2), count_empty_desc (2), end_plot (2), join_outputs (2), load_data (2), user_prompt (2), browse_metadata (1), map_metadata (1), map_metadata_compare (1), map_metadata_convert (1), user_categorisation_loop (1)

cli

cli_alert_info (17), cli_alert_danger (4), cli_alert_success (4), cli_h1 (3), cli_alert_warning (2)

stats

family (8), line (6), df (1)

graphics

title (7), legend (3), text (3)

dplyr

n (7), join_by (1), left_join (1)

utils

read.csv (7), data (1)

tidyr

complete (3)

gridExtra

grid.arrange (1), tableGrob (1)

htmlwidgets

saveWidget (2)

jsonlite

fromJSON (2)

plotly

plot_ly (2)

ggplot2

ggsave (1)

tools

file_path_sans_ext (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 11 files) and
1 authors
2 vignettes
5 internal data files
8 imported packages
17 exported functions (median 23 lines of code)
17 non-exported functions in R (median 24 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	11	60.2
files_vignettes	2	81.7
files_tests	18	93.9
loc_R	728	56.8
loc_vignettes	191	45.5
loc_tests	519	72.1
num_vignettes	2	85.5
data_size_total	24603	74.8
data_size_median	245	55.6
n_fns_r	34	43.4
n_fns_r_exported	17	62.2
n_fns_r_not_exported	17	35.5
n_fns_per_file_r	2	34.1
num_params_per_fn	3	29.4
loc_per_fn_r	24	68.0
loc_per_fn_r_exp	23	52.5
loc_per_fn_r_not_exp	24	70.6
rel_whitespace_R	19	58.8
rel_whitespace_vignettes	43	54.4
rel_whitespace_tests	25	74.9
doclines_per_fn_exp	18	11.1
doclines_per_fn_not_exp	0	0.0	TRUE
fn_call_network_size	31	54.8

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
12298710868	Auto Author Assign	success	57b719	72	2024-12-12
12293411548	auto-label	success	be2574	226	2024-12-12
12293835316	pages build and deployment	success	f8c518	121	2024-12-12
12293615596	pkgcheck	success	57b719	13	2024-12-12
12293809892	pkgdown	success	57b719	401	2024-12-12
12293615592	R-CMD-check.yaml	success	57b719	73	2024-12-12
12293615597	test-coverage.yaml	success	57b719	73	2024-12-12

3b. `goodpractice` results

`R CMD check` with rcmdcheck

R CMD check generated the following notes:

checking for portable file names ... NOTE
Found the following non-portable file paths:
browseMetadata/inst/outputs/L-OUTPUT_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-19-55.csv
browseMetadata/inst/outputs/LOG_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-19-55.csv
browseMetadata/inst/outputs/LOG_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-23-52.csv
browseMetadata/inst/outputs/OUTPUT_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-19-55.csv
browseMetadata/inst/outputs/OUTPUT_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-23-52.csv
browseMetadata/inst/outputs/PLOT_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-19-55.png
browseMetadata/inst/outputs/PLOT_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-11-27-14-23-52.png

Tarballs are only required to store paths of up to 100 bytes and cannot
store those of more than 256 bytes, with restrictions including to 100
bytes for the final component.
See section ‘Package structure’ in the ‘Writing R Extensions’ manual.
2. checking R code for possible problems ... NOTE
browse_metadata: no visible binding for global variable ‘Empty’
copy_previous: no visible binding for global variable ‘data_element’
count_empty_desc: no visible binding for global variable ‘empty’
end_plot: no visible binding for global variable ‘domain_code’
join_outputs: no visible binding for global variable ‘data_element’
map_metadata: no visible binding for global variable ‘note’
Undefined global functions or variables:
data_element domain_code empty Empty note

R CMD check generated the following check_fails:

no_import_package_as_a_whole
rcmdcheck_undefined_globals

Test coverage with covr

Package coverage: 89.15

Cyclocomplexity with cyclocomp

The following function have cyclocomplexity >= 15:

function	cyclocomplexity
map_metadata	18

Static code analyses with lintr

lintr found no issues with this package!

4. Other Checks

Details of other checks (click to open)

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

- browse_metadata from OECD

Package Versions

package	version
pkgstats	0.2.0.48
pkgcheck	0.1.2.77

Editor-in-Chief Instructions:

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

BatoolMM · 2024-12-12T14:57:05Z

Thank you soooo much @RayStick!!!

emilyriederer · 2024-12-14T02:34:10Z

Thanks for the submission, @RayStick ! This looks like a great candidate for rOpenSci. I'll begin the search for a handling editor

emilyriederer · 2024-12-14T02:34:43Z

@ropensci-review-bot seeking reviewers

ropensci-review-bot · 2024-12-14T02:34:46Z

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

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

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

maelle · 2024-12-16T06:48:38Z

@ropensci-review-bot assign @maelle as editor

ropensci-review-bot · 2024-12-16T06:48:41Z

Assigned! @maelle is now the editor

maelle · 2024-12-16T08:01:01Z

Editor checks:

Editor comments

Thank you for your submission! Below are some comments before I start looking for reviewers. Please feel free to ask me any question.

I think the DESCRIPTION and README should be more precise about the covered data sources, and their scope (countries, themes) and whether the package can be extended. That's my main concern at the moment.
Package name: are you sure you want to keep the name camel-case? We recommend lower-case names but at the same time I am aware that camel-case is better for accessibility so this is really just a question.
I'd recommend removing the "Maintainer" field from DESCRIPTION as Authors@R already covers it.
I'd recommend running desc::desc_normalize() to normalize DESCRIPTION. For instance the dependencies are not alphabetically ordered yet.
I like how welcoming the contributing guide is!
Instead of suggesting users start from an empty folder, could the functionality be integrated with for instance usethis::create_project()?
For opening an URL in the browser, you can recommend (or even add in a cli .run message) browseURL().
Why list internal functions in the pkgdown reference index?
In tests, individual test files do not have to load libraries. https://github.com/aim-rsf/browseMetadata/blob/main/tests/testthat/test-browse_metadata.R
Instead of tempdir() it is safer to use withr::local_tempfile() that automatically cleans up after itself. Context: https://testthat.r-lib.org/articles/test-fixtures.html (so you could get rid of https://github.com/aim-rsf/browseMetadata/blob/57b7191b967870160f06dcbec59fd58f36cbbb23/tests/testthat/test-browse_metadata.R#L32) -- that's personal preference though.
expect_true(nrow(bar_csv) == 26)) could be expect_equal(nrow(bar_csv), 26)). I like reading https://testthat.r-lib.org/reference/index.html#expectations once in a while because I easily forget about shorter expectations (and there are new ones). 😅 Actually expect_equal() is used in another file. Actually some linters might help you catch such cases: https://lintr.r-lib.org/reference/pkg_testthat_linters.html
For mocking you might like the new support for mocking in testthat https://testthat.r-lib.org/reference/local_mocked_bindings.html (see also https://blog.r-hub.io/2024/03/21/mocking-new-take/)
Regarding https://github.com/aim-rsf/browseMetadata/blob/57b7191b967870160f06dcbec59fd58f36cbbb23/R/map_metadata.R#L60 why not simply make getwd() the default for the output_dir argument? This way it's clearer to users what the default is.

maelle · 2024-12-16T08:03:53Z

I removed the seeking-reviewers label because I'll look for reviewers after your response @RayStick 🙂

RayStick · 2024-12-16T08:24:01Z

Thanks for these comments @maelle
This morning, I am going to add an rOpenSci review badge to my README, and add a NEWS file
Just to check - is the idea that I respond and implement your suggested changes above before it goes to review?

maelle · 2024-12-16T08:33:59Z

Ideally yes, either implementing or rejecting them (the first item on documenting scope is the most important one IMO) so the reviewers don't have to comment on the same components. Does it make sense? (it's a busy season so I expect things to take a while longer, no pressure)

RayStick · 2024-12-16T08:40:09Z

Thanks! Most seem pretty do-able to me, and I should have a bit of time today to go through them

Lextuga007 · 2025-01-29T09:02:55Z

@maelle thanks for sharing the blog - that's really useful for my own package development as I'm just puzzling out the best way to give messages and get them recorded for internal data pipeline packages😀

maurolepore · 2025-02-02T17:07:50Z

Dear @RayStick this is to mark the start of my EiC rotation. I'm reviewing all open issues and leaving a short note to myself of what I see.

I see both reviews are in and I assume you're working on incorporating the feedback. Everything seems on track. I'll step back. Thanks!

RayStick · 2025-02-03T16:08:23Z

Hi @maurolepore yes I am working on incorporating the feedback, thanks =D

RayStick · 2025-02-03T16:09:12Z

@maelle what is the etiquette with asking reviewers to review pull requests when I make some of their suggested changes? Is it best to just make the change to the best of my ability, and link to all the changes in one go at the end?

ropensci-review-bot · 2025-02-10T05:27:16Z

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

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

maelle · 2025-02-10T05:43:16Z

Is it best to just make the change to the best of my ability, and link to all the changes in one go at the end?

Sorry for the delay. Yes it is best to do it this way.

RayStick · 2025-02-10T16:53:21Z

@maelle I am aiming for end of this week to address all the reviewer's comments. Sorry for the delay, I have had a more hectic couple of work weeks than expected. Is that okay?

Progress is here: aim-rsf/mapmetadata#184 (but I will make to summarise and itemise in this issue thread as per the guidelines, once I am done)

maelle · 2025-02-11T07:54:47Z

Yes it is ok @RayStick, no worries.

Note that once you post your response, you can "submit" it with a bot command so the label of the issue will change: https://devguide.ropensci.org/bot_cheatsheet.html#submit-response-to-reviewers

RayStick · 2025-02-14T18:56:58Z

Hi there, thanks again for the brilliant reviews. I have addressed them all (see this issue) but I have not had a chance to do all my final sanity checks - sorry for the delay. On Monday (with some fresh eyes) I will do some final checks then write a message on this thread clearly describing how I responded to each of your queries (following the guidance given). Thank you for your patience. The changes took much longer than I thought!

maelle · 2025-02-17T06:25:42Z

👋 @RayStick! Thanks for the update and no worries. Have a good start to the week! ☕ 🍵

RayStick · 2025-02-17T19:33:33Z

My apologies, I was not too well today and after some user testing I found some bugs 😿 . Very close to finishing! Thanks for the patience.

maelle · 2025-02-18T10:31:40Z

Get well soon, take care!

RayStick · 2025-02-18T13:21:31Z

Response to reviewers

Please see the NEWS file and the new 4.0.0 release which incorporate all the changes. You can also see that pkgcheck results that were run on this 4.0.0 release showing everything passes

This GitHub issue is where I itemized all your feedback and made changes in grouped PRs - that's where you can see all the detail. Below I will try to summarise for you in a more digestible way!

Please let me know if you have any further questions/suggestions/corrections. Thank you again!

devchecks and lintr

This now has 0 errors, 0 warnings, and 1 note. The note is 'unable to verify current time' and I am not sure what to do about it! I also ran lintr::lint_package(path = ".") and implemented the suggestions. This resolves both @ymansiaux and @Lextuga007 comments about devchecks(). This also resolves @Lextuga007 comments about 'Package build' and 'Formatting consistency'

Improvement to package code and tests

As suggested by @Lextuga007:

Reduced the use of cat() and print() and allow suppression of some console messages (PR)
Removed some requests from the user for console input (some you had challenges with) to simplify (PR)
'data element' = 'variable', sorry. I have removed all mentioned of 'data element' in the code & docs (a legacy term) and only say 'variable' now (PR)
From reading your user testing comments about custom domain_list and lookup options, I realized I had over complicated this for a user. I have simplified the structure of these input files (PR) - this is all best explained by referencing the new README and package tutorial.

As suggested by @ymansiaux:

Removed the loading of packages in tests/testthat.R aim-rsf/mapmetadata@986a56a
- I struggled to replace my use of {mockery} with testthat::local_mocked_bindings() for one of the unit tests - is that okay? Perhaps we can put as an enhancement in a GitHub Issue.
Used file.path() for better compatibility across operating systems (PR)
Improved the clarity of arguments used in the map_convert() function (PR)
Implemented more extensive input validation (PR)

Smaller documentation improvements: formatting, re-naming, clarifications

As suggested by @Lextuga007:

Spelling corrections, improved accessibility of hyperlinks and explanation of technical language (PR).
Use the @family tag in function documentation, to group functions (PR) and now link to the function references throughout the tutorial.
When referencing the package data I now use ?mapmetadata::metadata and data(metadata) to make sure that users of this package (not in dev mode) can access these types of package files (PR)

As suggested by @ymansiaux:

Prevented .Rd files from being generated in the man/ folder by using the devtag as recommended by @maelle (aim-rsf/mapmetadata@d180590)
Removed the dontrun tags from the Examples and used if(interactive()) where needed. Also used temporary directories for better clean up (PR)

Larger documentation improvements

I have made lots of improvements to the README and the longer tutorial, expanding, clarifying and re-structuring. This responds to a few comments from both reviewers. I made sure that the README only includes the demo run of the main function, and all code snippets that a user is prompted to run can be copied & pasted without modification. The demo run is much shorter now, it processes the first 5 variables by default (not 20 like before). In the longer tutorial, I make it clearer to a wider range of package users how to access the example package files, and use these package files to demo the custom inputs. Implementing more extensive input validation (PR) and correcting some errors in the function docs also helps with user understanding I hope.

A few PRs but most notably - aim-rsf/mapmetadata#196 and aim-rsf/mapmetadata#199.

RayStick · 2025-02-18T13:21:57Z

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

ropensci-review-bot · 2025-02-18T13:22:04Z

Logged author response!

maelle · 2025-02-18T14:09:45Z

Thank you @RayStick!!

@ymansiaux @Lextuga007 could you please read @RayStick's response and answer using the template? Thank you!

maelle · 2025-02-18T14:10:10Z

@RayStick I'm intrigued by the mocking example. Where in the codebase is readline() introduced?

RayStick · 2025-02-18T14:46:39Z

@maelle yes it probably warrants another look! I am rather new to unit tests that use so much user interaction (before writing them for this package). Here is the line: https://github.com/aim-rsf/mapmetadata/blob/c6dcfa88bbe7666e810cefddf9b5344a0cef29de/R/user_interactions.R#L43

maelle · 2025-02-18T16:14:22Z

I have seen the line but I don't understand where readline() comes from (I can only find readline <- NULL so am missing something obvious I fear 😁 )

RayStick · 2025-02-18T16:36:16Z

Oh sorry - I understand your question now! readline() is base R, that is why I had to mock it (and also why you can't see it defined anywhere). Does that help with understanding?

maelle · 2025-02-18T17:22:23Z

ooooh it does, it was indeed obvious. 🤦 I'll try to have a look at the mocking tomorrow!

maelle · 2025-02-19T07:05:12Z

I made a suggestion in a PR aim-rsf/mapmetadata#207 using function factories instead of mockery. Also curious to hear whether @ymansiaux or @Lextuga007 have any experience with this. It's also something you could ask in the rOpenSci slack workspace.

EPI-YMX · 2025-02-19T11:07:02Z

Reviewer Response

Thank you for this high-quality work. I note that a great deal of work has gone into the documentation, which is now easier to handle.
Congratulations!

Final approval (post-review)

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

Estimated hours spent reviewing: 8

EPI-YMX · 2025-02-19T11:08:44Z

I made a suggestion in a PR aim-rsf/mapmetadata#207 using function factories instead of mockery. Also curious to hear whether @ymansiaux or @Lextuga007 have any experience with this. It's also something you could ask in the rOpenSci slack workspace.

Not really sorry. I remembered having used the trick readline <- NULL before, but your approach seems really interisting and more elegant !

RayStick · 2025-02-25T09:03:09Z

Use of mockery() now removed: aim-rsf/mapmetadata#207
Thanks @maelle =D

maelle · 2025-02-25T09:09:46Z

That was a neat exercise, getting me closer to understanding function factories 😁 😂

maelle · 2025-02-25T09:10:05Z

@Lextuga007 could you please read @RayStick's response and answer using the template? Thank you!

Lextuga007 · 2025-02-25T10:17:20Z

Apologies for the delay, I was off last week and will endeavour to do the review this week.

maelle · 2025-02-25T10:29:23Z

no worries, thank you!

RayStick changed the title ~~[WIP] Rachael Stickland~~ [WIP] browseMetadata Nov 26, 2024

RayStick closed this as completed Nov 26, 2024

RayStick reopened this Dec 12, 2024

RayStick changed the title ~~[WIP] browseMetadata~~ browseMetadata Dec 12, 2024

ropensci-review-bot added the 2/seeking-reviewer(s) label Dec 14, 2024

ropensci-review-bot assigned maelle Dec 16, 2024

ropensci-review-bot added the 1/editor-checks label Dec 16, 2024

maelle removed the 2/seeking-reviewer(s) label Dec 16, 2024

RayStick mentioned this issue Dec 16, 2024

R opensci editor comments aim-rsf/mapmetadata#167

Merged

16 tasks

RayStick mentioned this issue Feb 3, 2025

Responding to rOpenSci reviewer's comments aim-rsf/mapmetadata#184

Closed

16 tasks

mapmetadata #674

mapmetadata #674

Comments

RayStick commented Nov 26, 2024 • edited by ropensci-review-bot Loading

Scope

Technical checks

Publication options

Code of conduct

ropensci-review-bot commented Nov 26, 2024

ropensci-review-bot commented Nov 26, 2024

ropensci-review-bot commented Nov 26, 2024

Checks for browseMetadata (v2.0.1)

1. Package Dependencies

2. Statistical Properties

2a. Network visualisation

3. goodpractice and other checks

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:

RayStick commented Nov 26, 2024

mpadge commented Nov 27, 2024

RayStick commented Nov 27, 2024

RayStick commented Dec 12, 2024

ropensci-review-bot commented Dec 12, 2024

ropensci-review-bot commented Dec 12, 2024

ropensci-review-bot commented Dec 12, 2024

Checks for browseMetadata (v2.0.2)

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:

BatoolMM commented Dec 12, 2024

emilyriederer commented Dec 14, 2024

emilyriederer commented Dec 14, 2024

ropensci-review-bot commented Dec 14, 2024

maelle commented Dec 16, 2024

ropensci-review-bot commented Dec 16, 2024

maelle commented Dec 16, 2024 • edited Loading

Editor checks:

Editor comments

maelle commented Dec 16, 2024 • edited Loading

RayStick commented Dec 16, 2024 • edited Loading

maelle commented Dec 16, 2024

RayStick commented Dec 16, 2024

Lextuga007 commented Jan 29, 2025

maurolepore commented Feb 2, 2025

RayStick commented Feb 3, 2025

RayStick commented Feb 3, 2025 • edited Loading

ropensci-review-bot commented Feb 10, 2025

maelle commented Feb 10, 2025

RayStick commented Feb 10, 2025 • edited Loading

maelle commented Feb 11, 2025

RayStick commented Feb 14, 2025 • edited Loading

maelle commented Feb 17, 2025

RayStick commented Feb 17, 2025

maelle commented Feb 18, 2025

RayStick commented Feb 18, 2025

Response to reviewers

devchecks and lintr

Improvement to package code and tests

Smaller documentation improvements: formatting, re-naming, clarifications

Larger documentation improvements

RayStick commented Feb 18, 2025

ropensci-review-bot commented Feb 18, 2025

maelle commented Feb 18, 2025

maelle commented Feb 18, 2025

RayStick commented Feb 18, 2025

maelle commented Feb 18, 2025

RayStick commented Nov 26, 2024 •

edited by ropensci-review-bot

Loading

3. `goodpractice` and other checks

3b. `goodpractice` results

`R CMD check` with rcmdcheck

3. `goodpractice` and other checks

3b. `goodpractice` results

`R CMD check` with rcmdcheck

maelle commented Dec 16, 2024 •

edited

Loading

maelle commented Dec 16, 2024 •

edited

Loading

RayStick commented Dec 16, 2024 •

edited

Loading

RayStick commented Feb 3, 2025 •

edited

Loading

RayStick commented Feb 10, 2025 •

edited

Loading

RayStick commented Feb 14, 2025 •

edited

Loading

RayStick commented Feb 18, 2025 •

edited

Loading