Reduce Rd bloat from deep R6 inheritance chains - #1914
Open
kapelner wants to merge 1 commit into
Open
Conversation
In a package with a deep R6 inheritance chain (subclass -> parent ->
grandparent -> ...), every class's Rd page rendered a single flat
"Inherited methods" list mixing in every ancestor's methods together.
Since each level down the chain accumulates the methods of all its
ancestors, this list grows with total inherited method count across the
whole chain -- for a package with many R6 classes and inheritance depths
of 5+ levels, this produced Rd files with hundreds of lines per class
just for this one section, and (more importantly) made every one of
those classes' pages large enough that repeated Rd-to-text conversion
during R CMD INSTALL compounded a separate, unrelated R-core memory
issue we ran into and reported upstream.
This adds a new r6_inherited_documentation_display option (settable via
Config/roxygen2/r6_inherited_documentation_display in DESCRIPTION, or
man/roxygen/meta.R) with three values:
- "grouped" (new default): one collapsed subsection per ancestor class
that actually contributed an inherited method ("+ inherited public
methods from <ancestor>"), nearest ancestor first, each still listing
its own methods as linked bullets -- same information as before, but
organized by ancestor instead of flattened, and collapsed by default
instead of auto-expanding.
- "single": a single fixed-size line pointing at the immediate parent
class only, regardless of how many methods are inherited or how deep
the chain is -- the actual bloat fix, for packages where even the
grouped form is too large.
- "original": byte-for-byte the previous rendering, for anyone who
prefers it or hits a regression.
All three were verified against roxygen2's own test suite (8 new/updated
tests, plus updated snapshots for the two existing integration tests
that exercise multi-level R6 inheritance) and against a real-world
package with R6 inheritance chains up to 5 levels deep -- "original"
mode produced a total generated man/ size of ~5.0M, "grouped" reduced
that to ~4.1M (same information, just organized by ancestor and
collapsed by default instead of flattened and auto-expanded), and
"single" reduced it further to ~2.2M by dropping per-method enumeration
entirely.
This changes the default rendering (flat list -> grouped-by-ancestor,
collapsed instead of auto-expanded) but not the underlying information
shown; original is available for anyone who wants the old default back
exactly.
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.
In a package with a deep
R6inheritance chain (subclass -> parent -> grandparent -> ...), every class's Rd page rendered a single flat "Inherited methods" list mixing in every ancestor's methods together. Since each level down the chain accumulates the methods of all its ancestors, this list grows with total inherited method count across the whole chain -- for a package with many R6 classes and inheritance depths of 5+ levels, this produced Rd files with hundreds of lines per class just for this one section, and (more importantly) made every one of those classes' pages large enough that repeated Rd-to-text conversion duringR CMD INSTALLcompounded a separate, unrelated R-core memory issue we ran into and reported upstream.This adds a new
r6_inherited_documentation_displayoption (settable via Config/roxygen2/r6_inherited_documentation_display inDESCRIPTION, or man/roxygen/meta.R) with three values:All three were verified against
roxygen2's own test suite (8 new/updated tests, plus updated snapshots for the two existing integration tests that exercise multi-level R6 inheritance) and against a real-world package with R6 inheritance chains up to 5 levels deep -- "original" mode produced a total generated man/ size of ~5.0M, "grouped" reduced that to ~4.1M (same information, just organized by ancestor and collapsed by default instead of flattened and auto-expanded), and "single" reduced it further to ~2.2M by dropping per-method enumeration entirely.This changes the default rendering (flat list -> grouped-by-ancestor, collapsed instead of auto-expanded) but not the underlying information shown; original is available for anyone who wants the old default back exactly.