Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add function to remove environments tracked by ggplot2 #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vandenman
Copy link
Contributor

@vandenman vandenman commented Mar 10, 2022

@JorisGoosen this is an alternative/ global approach for https://github.com/jasp-stats/INTERNAL-jasp/issues/1755

Two things to consider:

  1. Implementing this globally might be tricky because it can definitely break existing plot code (see demoBad in inst/examples/ex-removeEnvironments.R). I'd argue that all code that breaks because of this is poorly written (and needs a rewrite).
  2. It's not super fast (yet). Right now, it recurses very deeply in ggplot2 structures. We can probably exit early and speed this up, but I haven't tested that yet. It's probably better to first check if this doesn't break too many figures in JASP.

To quickly test this in JASP you can change this line
https://github.com/jasp-stats/jaspResults/blob/49dcd41be799272b69ee562e496ee57afaaf33fa/R/zzzWrappers.R#L509
into

		plotObject  = function(x) if (missing(x)) private$jaspObject$plotObject   else private$jaspObject$plotObject   <- removeEnvironments(x),

and just dump the contents of R/removeEnvironments.R at the bottom of the file. Ugly, but otherwise you need to ensure that modules actually call the right jaspGraphs and all that...

@vandenman vandenman requested a review from JorisGoosen March 10, 2022 10:43
}

for (i in seq_along(x)) {
environment(x[[i]]) <- replacementEnv
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think of it, this part (and the other parts that remove environments) actually remove all environments. Perhaps I should add an option to only remove specified environments (by default ggplotObj$plot_env).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant