Skip to content

Conversation

@aaronw22
Copy link
Member

@aaronw22 aaronw22 commented Jan 2, 2026

To Do list

  1. Introduce canonical plot coercion
  • Add as_e61_plot() S3 generic
  • Methods for:
    • ggplot
    • e61_plot (idempotent)
    • (optional) patchwork, list
  • Ensure both theme61::ggplot() and save_e61() call as_e61_plot()
  1. Make “map vs non-map” a first-class concept
  • Introduce plot classes:

    • e61_plot
    • e61_map (inherits from e61_plot)
  • Decide classification strategy:

    • explicit (as_e61_map()), or
    • inferred once at save/preview time
  • Remove repeated if (is_spatial(...)) logic where possible

  1. Centralise save/preview branching with internal generics
  • Add internal generic(s), e.g.:
    • e61_prepare(plot, ...)
    • e61_plot_dims(plot, ...)
  • Implement methods:
    • e61_prepare.e61_plot
    • e61_prepare.e61_map
  • Refactor save_e61(), save_single(), save_multi() to call these generics instead of branching inline
  1. Convert ggplot helpers into spec objects using ggplot_add()
  • Refactor helpers such as:
    • plot_label()
    • add_baseline()
    • add_e61_logo()
    • geom_pointbar()
    • aes_y_lims(), aes_labs(), labs_e61()
  • Each helper returns a lightweight spec object
  • Implement ggplot_add.<spec_class>() methods
  • Allow behaviour to depend on e61_plot vs e61_map
  1. Unify theme_e61() and spatial theme logic
  • Make theme_e61() return an e61_theme spec object
  • Implement ggplot_add.e61_theme() that:
    • applies regular theme for e61_plot
    • applies spatial theme for e61_map
  • Optionally keep theme_e61_spatial() as a thin wrapper that just tags the plot as a map
  1. Refactor y-scale and secondary axis logic into S3 specs
  • Create spec classes (e.g. e61_y_scale, e61_secondary_axis)
  • Move complex branching out of:
    • scales.R
    • sec_axes_rescale.R
  • Handle map vs non-map differences via dispatch instead of conditionals
  1. Return structured results from save_e61()
  • Introduce an e61_save_result class
  • Include metadata (paths, dimensions, device, preview info)
  • Add methods:
    • print.e61_save_result()
    • as.character.e61_save_result()
    • (optional) knit_print.e61_save_result()

Describe your changes

Do the following before requesting a review

For feature branches

  • I have written tests covering functions I have added or changed.
  • I have run tests and they all pass.
  • I have ensured all new functions show up in the _pkgdown.yml file.
  • I have updated the package documentation with devtools::document().
  • I have updated DESCRIPTION with any new package dependencies.

Additional steps for the last PR into dev prior to releasing a new version from dev to main

  • I have updated DESCRIPTION with the new package version number.
  • I have updated NEWS.md with a brief description of my changes.
  • I have ensured the _pkgdown.yml file is correctly built by running pkgdown::check_pkgdown().
  • I have run devtools::build_readme() to update README.md.
  • I have updated the package website with pkgdown::build_site_github_pages().

# Conflicts:
#	man/setup_stadia_maps.Rd
# Conflicts:
#	man/setup_stadia_maps.Rd
#	tests/testthat/test-theme_e61.R
…vide them by default. Also changed the implementation for manual facet label position selection to be more streamlined.
…h a S3 method that calls at build time so that all the axes are detected regardless of how/where y = is defined + add tests"

This reverts commit 91f0d04.
…ot() with a S3 method that calls at build time so that all the axes are detected regardless of how/where y = is defined + add tests""

This reverts commit d0c8fb3.
…thod that calls at build time so that all the axes are detected regardless of how/where y = is defined + add tests
aaronw22 and others added 30 commits January 1, 2026 21:50
…into s3-rewrite

# Conflicts:
#	DESCRIPTION
# Conflicts:
#	R/ggplot_build-method.R
#	R/print-e61-ggplot.R
#	R/save-helpers.R
#	man/print.e61_plot.Rd
# Conflicts:
#	DESCRIPTION
#	NAMESPACE
#	R/plot_label.R
#	man/plot_label.Rd
#	tests/testthat/test-plot_label.R
# Conflicts:
#	DESCRIPTION
#	NAMESPACE
#	R/ggplot2-wrappers.R
#	R/ggplot_build-method.R
#	R/print-e61-ggplot.R
#	R/save-helpers.R
#	tests/testthat/test-ggplot2-wrappers.R
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.

2 participants