Prepare for rOpenSci peer review - #75
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prepares the whep R package for rOpenSci peer review by adding standard metadata files, updating the README, and creating a contributing guide. These are common requirements for rOpenSci submission.
Changes:
- Added
inst/CITATION,codemeta.json, and.pkgcheckfiles required for rOpenSci review and proper citation support. - Replaced the inline contributing instructions in both
README.RmdandREADME.mdwith a link to a new.github/CONTRIBUTING.mdguide, and added Citation and Code of Conduct sections. - Added a repostatus badge and excluded
codemeta.jsonfrom the R package build via.Rbuildignore.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
inst/CITATION |
New CITATION file with two entries: the whep package itself and the FABIO paper |
codemeta.json |
Auto-generated CodeMeta metadata for the package |
README.Rmd |
Updated with contributing link, citation section, code of conduct, and repostatus badge |
README.md |
Rendered version of README.Rmd with the same changes |
.github/CONTRIBUTING.md |
New contributing guide with code style, testing, and development workflow instructions |
.pkgcheck |
rOpenSci pkgcheck configuration disabling certain summary checks |
.Rbuildignore |
Added codemeta.json to build ignore list |
lbm364dl
force-pushed
the
lbm364dl/adapt-to-ropensci
branch
from
March 6, 2026 11:50
715e050 to
6998ed3
Compare
This reverts commit b34e7d8.
…ensci # Conflicts: # .Rbuildignore # .github/CONTRIBUTING.md # DESCRIPTION # R/gapfilling.R # R/scrape_faostat.R # README.Rmd # README.md # man/get_faostat_data.Rd
The March reflow of calculate_lmdi()'s roxygen block split two inline
code spans across lines, which roxygen renders as a literal space inside
the formula:
emissions:activity*(activity[sector]/activity)* (emissions[...]/...)
so the documented decomposition string no longer parses as written. The
reflow also did not buy anything: man/calculate_lmdi.Rd still has the
same four over-80-character lines either way, because the \code{} span
is emitted on one line regardless of how the source wraps.
The toy_examples.R rewrap and the any() wrapping in
test_scrape_faostat.R are dropped for the same reason -- the rewrapped
line was already under 80 characters, and .bad_activity_data_param_error()
returns a length-1 string, so any() guards nothing. Both were unrelated
to rOpenSci preparation.
CITATION.cff and codemeta.json are generated files that had drifted five months: they were written against a DESCRIPTION with 21 imports and three authors, and main now has 27 and five. Regenerated with cffr::cff_write() and codemetar::write_codemeta(), which also picks up the ERC funder, the repository topics as keywords, and per-author ORCIDs. inst/CITATION changes beyond regeneration: - The FABIO title now matches Crossref for 10.1021/acs.est.9b03554, which uses an em dash after FABIO and an en dash in Input-Output. Everything else in that entry was already correct, including Boerner. - The package entry gained the CRAN DOI and points at the pkgdown site rather than the bare repository. - The hardcoded year = "2025" is now taken from Date/Publication, so a release does not silently cite the wrong year. Source checkouts, where that field is unset, fall back to the current year. Not changed: citHeader(). The review comment asking for its removal cites a deprecation since R 4.3.0, but it is not deprecated -- on R 4.5.2 it is a plain wrapper around .bibentry(mheader=) and readCitationFile() parses the file without a warning.
Main grew a 393-line .github/CONTRIBUTING.md after this branch was cut, which left three problems the March version could not have anticipated. The README still summarised the workflow inline as seven bullets, so the detailed guide was unreachable from the front page. It now links there, using an absolute URL: a relative .github/CONTRIBUTING.md link resolves from the repository root on GitHub but 404s on the pkgdown site, which serves the README from a different depth. The two documents also disagreed on conduct. CONTRIBUTING told readers to report to the maintainers while this branch's new CODE_OF_CONDUCT.md adopted rOpenSci's, which is handled by their Code of Conduct Committee. CONTRIBUTING now defers to CODE_OF_CONDUCT.md and keeps only its own review etiquette, so there is a single reporting channel. CODE_OF_CONDUCT.md follows the shape of rOpenSci's own organisation default, which is a bilingual pointer rather than a copy of the text -- fitting for a CSIC project. The rOpenSci developer guide asks packages not transferred to their organisation to adopt that content, and asks CONTRIBUTING to reference it; both now do. The README paragraph is their recommended wording verbatim.
citation("whep") is user-visible output, so its move from the generated
DESCRIPTION default to two curated entries belongs in NEWS. The rest of
this branch is metadata and community files, which are not.
The References entry for Bruckner et al. (2019) carried 10.1021/acs.est.8b03704, which does not resolve -- it is absent from Crossref and returns 404 from doi.org. The FABIO paper's DOI is 10.1021/acs.est.9b03554, confirmed against Crossref (Environmental Science & Technology 53(19), 11302-11312) and already used correctly both at line 48 of this same vignette and in DESCRIPTION. Found by urlchecker::url_check(), which rOpenSci and CRAN both run. The same pass reports 403 for the corrected DOI, but that is ACS refusing automated HEAD requests, not a broken link. Borner is left unaccented: this vignette is deliberately pure ASCII.
R CMD check noted sovereign_iso3c as an undefined global in .attribute_dependency_land() and .dependency_sovereign_iso3(). CLAUDE.md requires every NSE variable be declared in utils::globalVariables(), so it is. The reason this survived is that check-r-package@v2 defaults to error-on: warning, so a NOTE never failed CI. .pkgcheck suppressed four summary items and three were misreadings: - ci_checks: pkgcheck reports "no continuous integration checks" when the commit being checked has no workflow runs on GitHub yet, which is what any local pre-push run looks like. The package has six workflows. - renv: renv was dropped from the package after this file was written, so there is nothing left to suppress. - coverage: suppressed while genuinely below rOpenSci's 75% threshold (currently 73.76%), which hid a submission blocker instead of closing it. Only num_imports stays, because it will not change -- 25 imports is what a pipeline over FAOSTAT, LUH2, LPJmL, CRU and HYDE costs, and pkgcheck marks it optional rather than failing.
This was referenced Aug 5, 2026
…ensci # Conflicts: # NEWS.md
…ensci # Conflicts: # NEWS.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the metadata and community files rOpenSci peer review expects, and brings
the branch back onto current
main— it was 1784 commits behind, so most ofthe work here was deciding what the March version still gets to keep.
What lands
inst/CITATIONcitation("whep")returned the generatedDESCRIPTIONdefault; now the package itself plus the FABIO paperCITATION.cffcffrcodemeta.jsoncodemetarreports no complaints.github/CODE_OF_CONDUCT.md.pkgcheckpkgchecksummary configurationREADMEBoth generated files had drifted five months: they described a package with 21
imports and three authors, against today's 25 and five.
Re-basing decisions worth a second pair of eyes
get_faostat_data()is kept. The branch's last commit removed it and theFAOSTATdependency as "broken".mainhas since gone the other way andhardened it twice —
e6922936gave it an offline fixture so the example runsunder
R CMD check, and4ebda44bfixed the ISO3 lazy-load side channel. Theupstream API is intact too:
FAOSTAT2.4.2 is on CRAN and still exportsget_faostat_bulk()andfillCountryCode(). Re-applying the removal would havereverted deliberate recent work, so it is reverted here instead.
main'sCONTRIBUTING.mdwins. The branch wrote an 86-line guide;mainhas since grown a 393-line one.
main's is kept as-is except its Conductsection, which told readers to report to the maintainers while this branch's new
CODE_OF_CONDUCT.mdadopts rOpenSci's, handled by their Code of ConductCommittee. Two policies with two reporting channels is exactly what a reviewer
would flag, so CONTRIBUTING now defers to the CoC file. The rOpenSci developer
guide asks for both that and the README wording, which is theirs verbatim.
The March doc reflows are dropped. Rewrapping
calculate_lmdi()'s roxygenblock split two inline code spans, and roxygen renders that as a literal space
inside the formula, so the documented decomposition string no longer parsed as
written:
It also bought nothing:
man/calculate_lmdi.Rdhas the same fourover-80-character lines either way, because the
\code{}span is emitted on oneline however the source wraps. The
toy_examples.Rrewrap and theany()wrapping in
test_scrape_faostat.Rgo with it — that line was already under 80characters, and
.bad_activity_data_param_error()returns a length-1 string, soany()guarded nothing.Two bugs found on the way
A DOI that does not resolve.
vignettes/footprint-analysis.RmdcitedBruckner et al. (2019) as
10.1021/acs.est.8b03704— absent from Crossref, 404from doi.org. The FABIO DOI is
10.1021/acs.est.9b03554, which the samevignette already uses correctly 550 lines earlier. Found by
urlchecker::url_check(), which rOpenSci and CRAN both run. (That pass alsoreports 403 for the corrected DOI, but that is ACS refusing automated requests,
not a broken link.)
An undeclared NSE variable.
R CMD checknotedsovereign_iso3cas anundefined global in
.attribute_dependency_land()and.dependency_sovereign_iso3(); it is now declared inutils::globalVariables()as CLAUDE.md requires. This is why it stayed hidden:
check-r-package@v2defaults to
error-on: warning, so a NOTE never failed CI.On the review comments
Two of the three Copilot comments were already fixed on the branch in
2a5ed30a(Börner's surname, in both
inst/CITATIONandcodemeta.json).The third asked to delete
citHeader()as "deprecated since R 4.3.0". It isnot: on R 4.5.2 it is a plain wrapper around
.bibentry(mheader=), andreadCitationFile()parses the file without a warning. Kept..pkgcheckis now honestIt suppressed four summary items; three were misreadings.
ci_checks— pkgcheck reports "no continuous integration checks" when thecommit it is checking has no workflow runs on GitHub yet, which is what every
local pre-push run looks like. The package has six workflows. Unsuppressed.
renv—renvwas dropped from the package after this file was written.Nothing left to suppress. Unsuppressed.
coverage— was suppressed while genuinely below the 75% threshold, hiding areal submission blocker. Unsuppressed, see below.
num_imports— kept. 25 imports is what a pipeline over FAOSTAT, LUH2,LPJmL, CRU and HYDE costs, and pkgcheck marks it optional rather than failing.
Where this leaves the rOpenSci checklist
pkgchecknow passes on contributing file, roxygen2, URL and BugReports fields,HTML vignette, citation file, license, and no
R CMD checkerrors orwarnings. Two blockers remain, both needing test and documentation work rather
than metadata, so they are not in this PR:
add_footprint_product_stage,add_polity_code,compute_footprint_paths,compute_fp_product_paths,create_alfredos_typologies,create_typologies_grafs_spain,create_typologies_of_josette,create_typologies_whep,get_polity_geometries,redistribute_feed.Informational, not blocking: cyclomatic complexity ≥ 15 in
build_constant_territory_series(31),resolve_polity_label(27),fill_linear(24) andbuild_io_model(19); and three exported names thatcollide with other CRAN packages (
fill_linearin transx,get_scopein FSK2R,read_populationin cape).Separately,
inst/DESCRIPTIONis a tracked copy of the DESCRIPTION from aroundv0.1.0.9000 — wrong version,
readxlin Imports, amipfpdependency thepackage no longer has, and two missing authors. It does not affect the installed
package (
R CMD INSTALLwrites the real one over it, verified), so it is deadweight rather than a bug, and removing it is left out of this PR.
Verification
citation("whep")renders both entries andtoBibtex()round-trips them.carries the em dash after FABIO and the en dash in Input–Output that the
published title uses.
Date/Publicationrather than ahardcoded
"2025", so a release cannot silently cite the wrong year.cffrvalidatesCITATION.cff;codemetarreports "the highest opinion ofthis R package".
lintr::lint_package()clean under the project's linter set,air format .clean, pkgdown reference index at parity with
man/(294 topics).README.mdregenerated fromREADME.Rmdwithgithub_document, not hand-edited.Closes #592. Part of #46.