Add show_warning argument

[jpquast]

This addresses issue #187.

I have the same problem with warnings from ggrepel showing up in unexpected places. I don't manage to suppress them either with SuppressWarnings(). Therefore, I added an additional argument to geom_text_repel() and geom_label_repel() called show_warning that allows the user to disable the display of warnings. The default is TRUE so that there is no change from the current behaviour.

[aphalo]

I have come across this warning rather frequently, but the simple solution in most cases is to increase max overlaps (Inf is accepted). There is parameter verbose already and show_warning does not seem to me informative enough of which warning is being silenced. In addition, in most cases I do not think that it is a good idea to drop labels from a plot silently, even at users' request (my own opinion...). Maybe a larger default value for max overlaps is a better solution, or even a default that depends on the total number of labels in a plot.

[jpquast]

The point is that I don't want to increase max.overlaps. I want to label what is possible with the default settings and drop the rest without the package generating a large amount of warnings. The way I implemented the show_warnings argument, all warnings are dropped from the two functions. In case of geom_label_repel() that is one warning and for geom_text_repel() that is two warnings. Ideally verbose would take over such a functionality, but it has a different use case for this package and is by default FALSE.

I am of course open to any modifications to my code suggestion as long as it would still allow the user to somehow silence warnings.

The problem for me is just that there is currently no way of silencing warnings of this package, which should be always possible. I use ggrepel in my package and the countless warnings from unit tests are always cluttering my checks. I would like to avoid this in some way or another.

[slowkow]

Hi @jpquast and thank you for the PR.

I'm not sure why suppressWarnings() doesn't work. I tried replacing rlang::warn() with warning() and I still can't suppress the warnings.

Here is a reprex that (sometimes) seems to reproduce the issue that was described in #187 :

library(ggrepel)

d <- mtcars
d$label <- rownames(d)

#' Here we get a warning that we expect.
p <- ggplot(d) +
  geom_point(aes(wt, mpg)) +
  geom_text_repel(aes(wt, mpg, label = label), size = 20)
p

#' This (sometimes) prints more than one warning.
p

#' This (sometimes) prints the warnings again, even though they were already printed.
set.seed(2)

#' This does not suppress the warnings, even though we would expect it to suppress them.
suppressWarnings({ p })

Sometimes, I get one warning, but sometimes I get more than one. I'm not sure how to reproducibly get multiple warnings.

I use ggrepel in my package and the countless warnings from unit tests are always cluttering my checks. I would like to avoid this in some way or another.

I can see why this would be a problem, and maybe the simplest workaround is the addition of the show_warning option. But I wonder if there is something we can do to make suppressWarnings() work as expected?

I'm a bit confused why we can't suppress these warnings. I have an inkling that maybe this has to do with the location of the call to warning() being inside the special function makeContent.textrepeltree() but I am not certain.

For me, assign("last.warning", NULL, envir = baseenv()) does not seem to do anything.

[jpquast]

Hi @slowkow, thanks for your reply!

the problem that your are describing is exactly the issue I have as well. I originally wanted to post a reprex, but it was simply not reproducible.

I unfortunately don't understand enough of geom constructor functions to know a better way to implement these warnings. The only solution that would work is to not even generate the warning as far as I can see for now.

In ggplot they generally seem to use cli::cli_warn (such as e.g. here), but not sure if that would make a difference. Also here the call is in the geom function and not a sub function.

[aphalo]

@slowkow If the pull request is accepted the parameter could be called warn_overlaps and default to TRUE, instead of show_warning. Of course, as @slowkow (Kamil) wrote it would be better to solve the problem at the root. Maybe @teunbrand (Teun van den Brand) can help diagnose the problem or give us some hint.

[teunbrand]

Not sure I understand the problem 100%, but here are some quick thoughts.

At least part of the problem why suppressWarnings(p) doesn't work is by because this just returns p, which will subsequently print to console and generate the warnings during printing and thus outside the suppressWarnings() part.

Secondly, the warnings are dynamically generated every time grid draws the figure, which delays these warnings beyond the print() function. So even if you use suppressWarnings(print(p)) explicitly, it'll throw the warnings whenever you resize the plotting window. This might only be a problem during interactive use though.

[slowkow]

the warnings are dynamically generated every time grid draws the figure

Thanks for the comment! This is consistent with what I was thinking. I tried some hacks, but I don't think there is a way to move the warning() calls into the geom_*() functions. So, we are stuck with warnings that are dynamically printed at draw-time (inside the makeContent.*() functions) where suppressWarnings(p) cannot reach them.

Therefore, I think the only way to address this is the way that @jpquast has done it, with an option to the geom_*() functions.

I feel that our usage of warn() (or warning()) inside the makeContent.*() functions is non-standard (different than every other package out there). So, maybe this behavior should be opt-in instead of opt-out.

Right now I am considering two options for how to proceed:

Option 1

Accept the PR by @jpquast to add an argument just for the warnings. I think I might suggest show_warnings or warnings instead of show_warning.

Considering that the warnings seem to be a nuisance for most users, I am starting to think that they should be off by default. Please share your thoughts. I want to avoid disturbing developers who depend on ggrepel.

Option 2

Use the verbose argument:

• verbose=TRUE will show messages about timing and warning messages about angle and overlaps
• verbose=FALSE will hide messages about timing and warning messages about angle and overlaps

And we'll set verbose=FALSE by default.

I feel like one argument (verbose) is simpler to understand for newcomers than two arguments (verbose and show_warnings).

I favor Option 2, but I'm willing to consider your thoughts @jpquast @aphalo @teunbrand

[aphalo]

@slowkow @jpquast I agree with Kamil: I like option 2 better than option 1. I suggest making the default verbose = getOption("verbose", default = FALSE) so that R's widely used option is available for debugging/additional information. R's help page for options describes this option as:

verbose:
logical. Should R report extra information on progress?

[jpquast]

@slowkow @aphalo I also agree with both of you. I think option 2 is the most intuitive one. At least I was hoping I could use verbose = FALSE to suppress the warnings when I first saw it existed. Then I figured out it had a different purpose in these functions.
Thanks for the discussions and willingness to change something regarding this issue!

[slowkow]

OK, I think I have a working version of the code that implements what we discussed:

master...verbose

[jpquast]

Great thanks! I tested it and it successfully removes the warnings in my package tests.