docs: document the R layer in ARCHITECTURE.md; correct the stale src … - #12
Merged
Conversation
…table
ARCHITECTURE.md named NONE of the five files in R/ -- it documented only
the C++ kernels, so a reader had no map of the layer they actually call.
The meta-repo's held-back audit check 8 (MR !76) reports all five.
Adds an "R contents" table in the same shape as the existing kernel
table, one row per file, covering what the R layer is for: the
validate / coerce / pad / dispatch / un-coerce sandwich in thin.R
(including why NA is rejected -- NA_integer_ is INT_MIN in the kernels,
where it corrupts neighbour sums far from the NA cell -- and why the
one-pixel pad exists), how medial_axis() differs from thin() (width
information), and distance_transform()'s all-foreground Inf convention.
R/RcppExports.R is documented as GENERATED by Rcpp::compileAttributes(),
matching how the existing table already documents src/RcppExports.cpp,
and lists the nine internal .Call shims plus the regenerate step. No
exemption for generated files is needed or proposed: the package's own
document already documents its generated .cpp counterpart, and the entry
carries information a reader needs.
Also corrects the adjacent kernel table, which was contradicted by the
new R rows. It listed Lee and K3M as "Stub" and omitted five files. All
seven methods are implemented and exercised by test-thin.R with exact
expected outputs; hilditch / opta / holt / medial_axis /
distance_transform / thinr_common.h were absent entirely. Design
constraint 3 ("Lee and K3M throw") is rewritten to the rule it was
really encoding -- unsupported input errors rather than silently falling
back -- which still holds for NA and >2-D arrays.
Docs-only. Verified with the real check-8 code extracted from the
figureextract meta-repo's origin/audit/architecture-contents-check:
thinr goes from 5 violations to 0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
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
1 participant
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.
…table
ARCHITECTURE.md named NONE of the five files in R/ -- it documented only the C++ kernels, so a reader had no map of the layer they actually call. The meta-repo's held-back audit check 8 (MR !76) reports all five.
Adds an "R contents" table in the same shape as the existing kernel table, one row per file, covering what the R layer is for: the validate / coerce / pad / dispatch / un-coerce sandwich in thin.R (including why NA is rejected -- NA_integer_ is INT_MIN in the kernels, where it corrupts neighbour sums far from the NA cell -- and why the one-pixel pad exists), how medial_axis() differs from thin() (width information), and distance_transform()'s all-foreground Inf convention.
R/RcppExports.R is documented as GENERATED by Rcpp::compileAttributes(), matching how the existing table already documents src/RcppExports.cpp, and lists the nine internal .Call shims plus the regenerate step. No exemption for generated files is needed or proposed: the package's own document already documents its generated .cpp counterpart, and the entry carries information a reader needs.
Also corrects the adjacent kernel table, which was contradicted by the new R rows. It listed Lee and K3M as "Stub" and omitted five files. All seven methods are implemented and exercised by test-thin.R with exact expected outputs; hilditch / opta / holt / medial_axis / distance_transform / thinr_common.h were absent entirely. Design constraint 3 ("Lee and K3M throw") is rewritten to the rule it was really encoding -- unsupported input errors rather than silently falling back -- which still holds for NA and >2-D arrays.
Docs-only. Verified with the real check-8 code extracted from the figureextract meta-repo's origin/audit/architecture-contents-check: thinr goes from 5 violations to 0.