-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
mapmetadata #674
Comments
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
🚀 Editor check started 👋 |
Checks for browseMetadata (v2.0.1)git hash: 3a779939
Important: All failing checks above must be addressed prior to proceeding (Checks marked with 👀 may be optionally addressed.) Package License: GPL (>= 3) 1. Package DependenciesDetails 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.
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. baselist (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) browseMetadatajson_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) clicli_alert_info (17), cli_alert_danger (4), cli_alert_success (4), cli_h1 (3), cli_alert_warning (2) statsfamily (8), line (6), df (1) graphicstitle (7), legend (3), text (3) dplyrn (7), join_by (1), left_join (1) utilsread.csv (7), data (1) tidyrcomplete (3) gridExtragrid.arrange (1), tableGrob (1) htmlwidgetssaveWidget (2) jsonlitefromJSON (2) plotlyplot_ly (2) ggplot2ggsave (1) toolsfile_path_sans_ext (1) 2. Statistical PropertiesThis 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:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
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.
Closing issue for now, as I misread some recommendations for a package to be ready. Will address these properly (the |
@RayStick No worries. When you're ready, please just open this issue again (and not a new issue), call |
Okay I will do that, thanks @mpadge! |
@ropensci-review-bot check package |
Thanks, about to send the query. |
🚀 Editor check started 👋 |
Checks for browseMetadata (v2.0.2)git hash: 57b7191b
(Checks marked with 👀 may be optionally addressed.) Package License: GPL (>= 3) 1. Package DependenciesDetails 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.
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. baselist (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) browseMetadatajson_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) clicli_alert_info (17), cli_alert_danger (4), cli_alert_success (4), cli_h1 (3), cli_alert_warning (2) statsfamily (8), line (6), df (1) graphicstitle (7), legend (3), text (3) dplyrn (7), join_by (1), left_join (1) utilsread.csv (7), data (1) tidyrcomplete (3) gridExtragrid.arrange (1), tableGrob (1) htmlwidgetssaveWidget (2) jsonlitefromJSON (2) plotlyplot_ly (2) ggplot2ggsave (1) toolsfile_path_sans_ext (1) 2. Statistical PropertiesThis 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:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
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
Thank you soooo much @RayStick!!! |
Thanks for the submission, @RayStick ! This looks like a great candidate for rOpenSci. I'll begin the search for a handling editor |
@ropensci-review-bot seeking reviewers |
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 |
@ropensci-review-bot assign @maelle as editor |
Assigned! @maelle is now the editor |
Editor checks:
Editor commentsThank you for your submission! Below are some comments before I start looking for reviewers. Please feel free to ask me any question.
|
I removed the seeking-reviewers label because I'll look for reviewers after your response @RayStick 🙂 |
Thanks for these comments @maelle |
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) |
Thanks! Most seem pretty do-able to me, and I should have a bit of time today to go through them |
@ymansiaux: If you haven't done so, please fill this form for us to update our reviewers records. |
@ropensci-review-bot add @Lextuga007 to reviewers |
@Lextuga007 added to the reviewers list. Review due date is 2025-01-30. Thanks @Lextuga007 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. |
@Lextuga007: If you haven't done so, please fill this form for us to update our reviewers records. |
@ymansiaux @Lextuga007 thanks so much to you both for accepting to review this package 🙏 |
Hi @maelle and @RayStick , please find below my review. Package Review
DocumentationThe package includes all the following forms of documentation:
Functionality
Please see below my review comments. I think that some modifications should be made, for example in the README or function examples. Estimated hours spent reviewing: 7
Review CommentsThank you very much for sharing this work and for your high-quality development. I particularly appreciate the attention you've paid to documentation and unit testing. I’ve personally learned a lot, especially about mocking, which I hadn’t explored much until now. Below, please find my detailed comments and suggestions: Function and argument naming To align with the naming of the other exported functions, For the Input checking I suggest adding explicit input validation to your exported functions. This would enhance the user experience by providing clear and informative error messages or warnings.
README The demo example in the README feels somewhat extensive and might benefit from being simplified to improve accessibility. Currently, interacting with up to 20 variables requires substantial effort, which could deter users from running the full demo. For sections involving custom inputs (metadata input, domain list input, and custom lookup), I recommend moving these to the vignette and referencing it in the README. For example:
Additionally, examples in the README should be copy-pasteable and work seamlessly without requiring additional setup. For instance, the following code snippet requires downloading a file. new_csv_file <- "path/your_new_csv.csv"
demo_domains_file <- system.file("inputs/domain_list_demo.csv", package = "mapmetadata")
metadata_map(csv_file = new_csv_file, domain_file = demo_domains_file) This is why I would present only the demo example in the README. Providing examples for custom domain list inputs and custom lookup tables would also be beneficial. For these examples, linking to demo files in Documentation For non-exported functions, you might want to prevent Testing In the A note regarding Examples Some examples are enclosed in
# Locate file paths for the example files in the package
demo_session_dir <- system.file("outputs", package = "mapmetadata")
demo_session1_base <- "360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55"
demo_session2_base <- "360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45"
demo_metadata_file <- system.file("inputs", "360_NationalCommunityChildHealthDatabase(NCCHD)_Structural_Metadata.csv", package = "mapmetadata")
demo_domain_file <- system.file("inputs", "domain_list_demo.csv", package = "mapmetadata")
output_dir <- tempdir()
message("Output directory: ", output_dir)
# Run the function - requires user interaction
map_compare(
session_dir = demo_session_dir,
session1_base = demo_session1_base,
session2_base = demo_session2_base,
metadata_file = demo_metadata_file,
domain_file = demo_domain_file,
output_dir = output_dir
)
Returning the path of the output file and including a — The example for — For #' @examples
# Demo run requires no function inputs but requires user interaction
if(interactive()) {
metadata_map()
} file path construction In your file-handling functions, you are constructing paths using You could replace csv_1a <- read.csv(paste0(session_dir, "/", "MAPPING_LOG_", session1_base, ".csv")) with csv_1a <- read.csv(file.path(session_dir, paste0("MAPPING_LOG_", session1_base, ".csv"))) This ensures compatibility across operating systems. devtools::check() Here is my session info sessionInfo()
R version 4.4.1 (2024-06-14 ucrt)
Platform: x86_64-w64-mingw32/x64
Running under: Windows 11 x64 (build 22631)
Matrix products: default
locale:
[1] LC_COLLATE=French_France.utf8 LC_CTYPE=French_France.utf8
[3] LC_MONETARY=French_France.utf8 LC_NUMERIC=C
[5] LC_TIME=French_France.utf8
time zone: Europe/Paris
tzcode source: internal
attached base packages:
[1] stats graphics grDevices utils datasets methods base
other attached packages:
[1] mapmetadata_3.0.0 testthat_3.2.2
loaded via a namespace (and not attached):
[1] gtable_0.3.6 jsonlite_1.8.9 dplyr_1.1.4
[4] compiler_4.4.1 brio_1.1.5 tidyselect_1.2.1
[7] gridExtra_2.3 tidyr_1.3.1 scales_1.3.0
[10] yaml_2.3.10 fastmap_1.2.0 ggplot2_3.5.1
[13] R6_2.5.1 generics_0.1.3 knitr_1.49
[16] htmlwidgets_1.6.4 tibble_3.2.1 desc_1.4.3
[19] munsell_0.5.1 rprojroot_2.0.4 pillar_1.10.1
[22] rlang_1.1.4.9000 xfun_0.50 fs_1.6.5
[25] lazyeval_0.2.2 pkgload_1.4.0 viridisLite_0.4.2
[28] plotly_4.10.4 cli_3.6.3.9000 withr_3.0.2
[31] magrittr_2.0.3 crosstalk_1.2.1 digest_0.6.37
[34] grid_4.4.1 rstudioapi_0.17.1 lifecycle_1.0.4
[37] vctrs_0.6.5 evaluate_1.0.3 glue_1.8.0
[40] data.table_1.16.4 farver_2.1.2 pkgbuild_1.4.5
[43] colorspace_2.1-1 rmarkdown_2.29 purrr_1.0.2
[46] httr_1.4.7 tools_4.4.1 pkgconfig_2.0.3
[49] htmltools_0.5.8.1 Below is the result of devtools::check(document = FALSE)
══ Building ═════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Setting env vars:
• CFLAGS : -Wall -pedantic -fdiagnostics-color=always
• CXXFLAGS : -Wall -pedantic -fdiagnostics-color=always
• CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX14FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX17FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX20FLAGS: -Wall -pedantic -fdiagnostics-color=always
── R CMD build ──────────────────────────────────────────────────────────────────────────────────────────────────────────────
✔ checking for file 'C:\Users\yohann\Documents\code\reviewropensci\mapmetadata/DESCRIPTION' (454ms)
─ preparing 'mapmetadata': (1.7s)
✔ checking DESCRIPTION meta-information ...
─ installing the package to build vignettes
✔ creating vignettes (7.3s)
─ checking for LF line-endings in source and make files and shell scripts (891ms)
─ checking for empty or unneeded directories
─ building 'mapmetadata_3.0.0.tar.gz'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/BAR_360_NationalCommunityChildHealthDatabase(NCCHD)_2024-12-19-14-11-55.html'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/CONSENSUS_MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-38-45.csv'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/L-MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/MAPPING_LOG_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/MAPPING_LOG_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv'
Avis dans utils::tar(filepath, pkgname, compression = compression, compression_level = 9L, :
storing paths of more than 100 bytes is not portable:
'mapmetadata/inst/outputs/MAPPING_PLOT_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.png'
══ Checking ═════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Setting env vars:
• _R_CHECK_CRAN_INCOMING_REMOTE_ : FALSE
• _R_CHECK_CRAN_INCOMING_ : FALSE
• _R_CHECK_FORCE_SUGGESTS_ : FALSE
• _R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_: FALSE
• NOT_CRAN : true
── R CMD check ──────────────────────────────────────────────────────────────────────────────────────────────────────────────
─ using log directory 'C:/Users/yohann/Documents/code/reviewropensci/mapmetadata.Rcheck' (480ms)
─ using R version 4.4.1 (2024-06-14 ucrt)
─ using platform: x86_64-w64-mingw32
─ R was compiled by
gcc.exe (GCC) 13.2.0
GNU Fortran (GCC) 13.2.0
─ running under: Windows 11 x64 (build 22631)
─ using session charset: UTF-8
─ using options '--no-manual --as-cran' (858ms)
✔ checking for file 'mapmetadata/DESCRIPTION'
─ checking extension type ... Package
─ this is package 'mapmetadata' version '3.0.0'
─ package encoding: UTF-8
✔ checking package namespace information
✔ checking package dependencies (1.2s)
✔ checking if this is a source package ...
✔ checking if there is a namespace
✔ checking for executable files (926ms)
✔ checking for hidden files and directories ...
N checking for portable file names
Found the following non-portable file paths:
mapmetadata/inst/outputs/BAR_360_NationalCommunityChildHealthDatabase(NCCHD)_2024-12-19-14-11-55.html
mapmetadata/inst/outputs/CONSENSUS_MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-38-45.csv
mapmetadata/inst/outputs/L-MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv
mapmetadata/inst/outputs/MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv
mapmetadata/inst/outputs/MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv
mapmetadata/inst/outputs/MAPPING_LOG_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv
mapmetadata/inst/outputs/MAPPING_LOG_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv
mapmetadata/inst/outputs/MAPPING_PLOT_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.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.
✔ checking serialization versions
✔ checking whether package 'mapmetadata' can be installed (9.6s)
N checking installed package size ...
installed size is 5.3Mb
sub-directories of 1Mb or more:
outputs 4.4Mb
✔ checking package directory (357ms)
N checking for future file timestamps (4.2s)
unable to verify current time
✔ checking 'build' directory
✔ checking DESCRIPTION meta-information (349ms)
✔ checking top-level files
✔ checking for left-over files ...
✔ checking index information ...
✔ checking package subdirectories (1s)
✔ checking code files for non-ASCII characters ...
✔ checking R files for syntax errors ...
✔ checking whether the package can be loaded (994ms)
✔ checking whether the package can be loaded with stated dependencies (706ms)
✔ checking whether the package can be unloaded cleanly (780ms)
✔ checking whether the namespace can be loaded with stated dependencies (788ms)
✔ checking whether the namespace can be unloaded cleanly (1.1s)
✔ checking loading without being on the library search path (1.2s)
✔ checking dependencies in R code (2s)
✔ checking S3 generic/method consistency (1.1s)
✔ checking replacement functions (892ms)
✔ checking foreign function calls (790ms)
N checking R code for possible problems (6.2s)
empty_count: no visible binding for global variable 'Section'
empty_count: no visible binding for global variable
'Column.description'
empty_count: no visible binding for global variable 'total_variables'
empty_count: no visible binding for global variable 'no_count'
empty_count: no visible binding for global variable 'Empty'
end_plot: no visible binding for global variable 'domain_code'
map_compare: no visible binding for global variable 'Section'
map_compare: no visible binding for global variable 'data_element'
map_compare: no visible global function definition for 'contains'
metadata_map: no visible binding for global variable 'Section'
metadata_map: no visible binding for global variable 'note'
output_copy: no visible binding for global variable 'data_element'
Undefined global functions or variables:
Column.description Empty Section contains data_element domain_code
no_count note total_variables
✔ checking Rd files (571ms)
✔ checking Rd metadata ...
N checking Rd line widths ...
Rd file 'map_compare.Rd':
\examples lines wider than 100 characters:
demo_metadata_file <- system.file("inputs", "360_NationalCommunityChildHealthDatabase(NCCHD)_Structural_Metadata.csv", package = "mapme ... [TRUNCATED]
Rd file 'map_convert.Rd':
\examples lines wider than 100 characters:
demo_output_csv <- "MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv"
These lines will be truncated in the PDF manual.
✔ checking Rd cross-references ...
✔ checking for missing documentation entries (925ms)
✔ checking for code/documentation mismatches (2.4s)
✔ checking Rd \usage sections (1.2s)
✔ checking Rd contents ...
✔ checking for unstated dependencies in examples ...
✔ checking contents of 'data' directory
✔ checking data for non-ASCII characters ...
✔ checking LazyData
✔ checking data for ASCII and uncompressed saves ...
✔ checking installed files from 'inst/doc' ...
✔ checking files in 'vignettes' ...
✔ checking examples (1.5s)
✔ checking for unstated dependencies in 'tests' ...
─ checking tests ...
✔ Running 'testthat.R' (4.7s)
✔ checking for unstated dependencies in vignettes (563ms)
✔ checking package vignettes ...
✔ checking re-building of vignette outputs (4.9s)
✔ checking for non-standard things in the check directory ...
✔ checking for detritus in the temp directory
See
'C:/Users/yohann/Documents/code/reviewropensci/mapmetadata.Rcheck/00check.log'
for details.
── R CMD check results ─────────────────────────────────────────────────────────────────────────────── mapmetadata 3.0.0 ────
Duration: 1m 0.1s
❯ checking for portable file names ... NOTE
Found the following non-portable file paths:
mapmetadata/inst/outputs/BAR_360_NationalCommunityChildHealthDatabase(NCCHD)_2024-12-19-14-11-55.html
mapmetadata/inst/outputs/CONSENSUS_MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-38-45.csv
mapmetadata/inst/outputs/L-MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv
mapmetadata/inst/outputs/MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv
mapmetadata/inst/outputs/MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv
mapmetadata/inst/outputs/MAPPING_LOG_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.csv
mapmetadata/inst/outputs/MAPPING_LOG_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv
mapmetadata/inst/outputs/MAPPING_PLOT_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-11-55.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.
❯ checking installed package size ... NOTE
installed size is 5.3Mb
sub-directories of 1Mb or more:
outputs 4.4Mb
❯ checking for future file timestamps ... NOTE
unable to verify current time
❯ checking R code for possible problems ... NOTE
empty_count: no visible binding for global variable 'Section'
empty_count: no visible binding for global variable
'Column.description'
empty_count: no visible binding for global variable 'total_variables'
empty_count: no visible binding for global variable 'no_count'
empty_count: no visible binding for global variable 'Empty'
end_plot: no visible binding for global variable 'domain_code'
map_compare: no visible binding for global variable 'Section'
map_compare: no visible binding for global variable 'data_element'
map_compare: no visible global function definition for 'contains'
metadata_map: no visible binding for global variable 'Section'
metadata_map: no visible binding for global variable 'note'
output_copy: no visible binding for global variable 'data_element'
Undefined global functions or variables:
Column.description Empty Section contains data_element domain_code
no_count note total_variables
❯ checking Rd line widths ... NOTE
Rd file 'map_compare.Rd':
\examples lines wider than 100 characters:
demo_metadata_file <- system.file("inputs", "360_NationalCommunityChildHealthDatabase(NCCHD)_Structural_Metadata.csv", package = "mapme ... [TRUNCATED]
Rd file 'map_convert.Rd':
\examples lines wider than 100 characters:
demo_output_csv <- "MAPPING_360_NationalCommunityChildHealthDatabase(NCCHD)_CHILD_2024-12-19-14-17-45.csv"
These lines will be truncated in the PDF manual.
0 errors ✔ | 0 warnings ✔ | 5 notes ✖
R CMD check succeeded
Here are my notes regarding the five
demo_metadata_file <- system.file(
"inputs",
"360_NationalCommunityChildHealthDatabase(NCCHD)_Structural_Metadata.csv",
package = "mapmetadata"
) I hope these comments are helpful! Please don’t hesitate to reach out if anything requires further clarification. Best regards, Yohann |
Thank you so much for your comments @ymansiaux ! I won’t work on them today (or until the other review comes in) but I have a quick question -
I struggled with function naming and changed them a lot 😅
Does that make sense? I am open to changing them! On reflection, I am using map to be an object and a verb so that is confusing. |
Yes that perfectly makes sense :-) Considering the object_verb() naming scheme I think you should keep the current function names. Have a good day ! |
Thanks so much @ymansiaux for your thorough review! |
@ropensci-review-bot submit review #674 (comment) time 7 |
Logged review for ymansiaux (hours: 7) |
As an alternative to |
📆 @Lextuga007 you have 2 days left before the due date for your review (2025-01-30). |
Just to start by saying thank you for all the work you've put into this package 😃 Like @ymansiaux I learned a lot from the review and I hope my points are also helpful. Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Some changes are required, particularly for README and vignette documentation. Estimated hours spent reviewing: 6.5 hours
Review CommentsComprehensive tests for functions (both exported and internal) with every function having a test. ROpenSci packaging guidelinesConsole messages asks to avoid the use of Avoiding verbosity or suppressing messages is hard within this package as the interactions from the user require the messages. AccessibilityThank you for the alt text on images! To support further accessibility I recommend being more descriptive in the text where a link is highlighted in text like for the word Also appears in the vignette which shows an image but only says It took me a while to navigate the website as I wasn't familiar with it so I wonder if the image would benefit from a direct hyperlink to the same page? And the non-descriptive Technical languageIt might be useful to expand on the point about changing the save location by adjusting the Expectations of functionalityOnly the Return key works to continue where the instruction says to The demo requires initials to be entered but it isn't explained why it is required. Demo mode is said to process only the first 20 variables The parameters used in the
For the prompt I recommend linking to the function reference for I recommend being explicit in the argument help for metadata_file as The reference to For For Function documentation sometimes includes references to other functions in the package, for example SpellingMinor spelling have been submitted for correction on a PR but where functions have spelling mistakes:
It is also in the message from the Object in function Package buildNotes on
Formatting consistencyI noticed that there are line breaks put into function @ymansiaux shared the devtools::check output which is useful as I don't get all the same notes (I'm missing the line widths note) so for comparison this is mine:
And my session info:
|
Thanks a lot @Lextuga007 for your thorough review! 🚀 Regarding verbosity, we've published a blog post a while ago (with the goal of updating the dev guide later), that might be relevant: https://ropensci.org/blog/2024/02/06/verbosity-control-packages/ @RayStick Both reviews are now in. 😸 |
@ropensci-review-bot submit review #674 (comment) time 6.5 |
Logged review for Lextuga007 (hours: 6.5) |
@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😀 |
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! |
Hi @maurolepore yes I am working on incorporating the feedback, thanks =D |
@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? |
@RayStick, @BatoolMM, @Rainiefantasy: please post your response with Here's the author guide for response. https://devguide.ropensci.org/authors-guide.html |
Sorry for the delay. Yes it is best to do it this way. |
@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) |
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 |
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
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.):
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.
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.
Not that I am aware of
Not Applicable
Not Applicable
pkgcheck
items which your package is unable to pass.Technical checks
Confirm each of the following by checking the box.
This package:
Publication options
Do you intend for this package to go on CRAN?
Do you intend for this package to go on Bioconductor?
Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: