Skip to content

docs: stop the ggproto() example from writing to fields - #6854

Open
CuiweiG wants to merge 4 commits into
tidyverse:mainfrom
CuiweiG:docs/issue-6582-ggproto-example
Open

CuiweiG wants to merge 4 commits into
tidyverse:mainfrom
CuiweiG:docs/issue-6582-ggproto-example

Conversation

@CuiweiG

@CuiweiG CuiweiG commented Apr 22, 2026 •

Copy link
Copy Markdown
Contributor

Closes #6582.

The ?ggproto example assigned to a field inside a method (self$x <- self$x + n), which is the pattern extension authors should avoid. This keeps the Adder/Doubler example, but add() now only reads self$x. Doubler overrides x, so the example also shows that self is still the child when the parent's method is called through ggproto_parent() (Doubler$add(10) gives 25).

The "Working with ggproto classes" section gets one extra sentence: reading fields through self is fine, assigning to them is discouraged.

The prior @examples for ggproto() used stateful classes (Adder$x),
a pattern discouraged for extensions since most ggplot2 layer classes
are stateless. The new Math/Mather example covers construction, method
dispatch and ggproto_parent() without `self$*` access, and the
Working-with-classes section adds a paragraph on the stateless norm.

Closes tidyverse#6582.
@teunbrand

Copy link
Copy Markdown
Collaborator

Thanks for preparing a PR!
I think the proposed change is not exactly what we want users to learn about ggproto classes. It may have very well be my insufficient explanation in #6582 that had set this on the wrong track.
To explain a bit more; it is fine to read from a ggproto object's fields, but to write to that field is discouraged.
In the old example, self$x <- self$x + n is the problematic piece.

@CuiweiG

CuiweiG commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification — the write-vs-read distinction wasn't clear to me from the issue alone. Will revise the example to keep the field-based inheritance pattern but drop the mutation. Back to you shortly.

Per review in tidyverse#6854: reading fields through self is fine, assigning to them is what should be discouraged. Restore the Adder/Doubler example without the in-place update, let Doubler override x so the example shows that self is still the child inside ggproto_parent(), and replace the 'stateless classes' paragraph with one sentence on read vs write.
ggplot2's own methods assign to self fields on per-plot objects, so use the wording from the review instead of a flat rule. Also fix the NEWS entry, which read as if methods were required to read fields.
@CuiweiG CuiweiG changed the title docs: replace stateful ggproto example with stateless one docs: stop the ggproto() example from writing to fields Oct 23, 2026
@CuiweiG

CuiweiG commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for the long wait on this one.

Reworked it based on your explanation. The Adder/Doubler example is back, but add() now only reads self$x and doesn't assign to it. I also gave Doubler its own x = 5, so Doubler$add(10) returns 25. That way the example shows self is still the child when the parent method is called via ggproto_parent().

I removed the paragraph about stateless classes and added just one sentence under "Working with ggproto classes": reading fields through self is fine, but assigning to them is discouraged. NEWS is updated and I merged in main.

If you'd rather keep Doubler as it was (without overriding x), I can change it back.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Example in ?ggproto counterproducive

2 participants