Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

awkward behavior from scale_fill_viridis_c(trans = "log") #4185

Closed
twest820 opened this issue Aug 28, 2020 · 3 comments · Fixed by #5343
Closed

awkward behavior from scale_fill_viridis_c(trans = "log") #4185

twest820 opened this issue Aug 28, 2020 · 3 comments · Fixed by #5343
Labels
messages requests for improvements to error, warning, or feedback messages

Comments

@twest820
Copy link

twest820 commented Aug 28, 2020

Three interrelated issues:

  1. Log transformation of zero values for a continuous fill produces a warning about infinite values in a discrete y axis. This warning could be more clearly worded since the fill is neither discrete nor a y axis. There also appears to be differentiation of NaN from -Inf, leading to a cryptic NaN warning from negative fill values.
  2. There doesn't appear to be any way to tell ggplot these fill values are expected in order to suppress the warnings. The closest I can see is something like scale_fill_viridis_c(na.value = "transparent", trans = "log"), which plots fine since apparently -Inf, NaN, and NA are treated interchangeably but is orthogonal to warning generation.
  3. The resulting labels on the fill colorbar include 8+ decimal places, which is a curiously verbose default, though format_format(digits = n) provides a simple mitigation.
library(ggplot2)
library(magrittr)
library(scales)
library(tidyverse)

data = crossing(x = seq(0, 1, length.out = 100), y = seq(0, 1, length.out = 100))
data %<>% mutate(value = x - y)
ggplot(data) + geom_raster(aes(x = x, y = y, fill = value)) + scale_fill_viridis_c(trans = "log")

Warning messages:
1: In self$trans$transform(x) : NaNs produced
2: Transformation introduced infinite values in discrete y-axis 

# change for more concise labeling
ggplot(data) + geom_raster(aes(x = x, y = y, fill = value)) + scale_fill_viridis_c(labels = format_format(digits = 2), trans = "log")

Observed on R 4.0.2 with tidyverse 1.3.0 (ggplot 3.3.2). I suspect this might also occur with other scale_fill functions.
ggplot scale_fill_viridis_c

@EmilHvitfeldt
Copy link

Some of the issues you are getting are related to what happens when you take logarithms of non-positive numbers.

seq(-1, 1, by = 0.5)
#> [1] -1.0 -0.5  0.0  0.5  1.0

log(seq(-1, 1, by = 0.5))
#> Warning in log(seq(-1, 1, by = 0.5)): NaNs produced
#> [1]        NaN        NaN       -Inf -0.6931472  0.0000000

The NaN's appear because logarithms for negative numbers are undefined. NaN is generally treated differently than NA.
We are getting infinite values after transformation since log(0) = -Inf.

@thomasp85 thomasp85 added the messages requests for improvements to error, warning, or feedback messages label Mar 25, 2021
@teunbrand
Copy link
Collaborator

I also ran into this and I think these lines are responsible:

ggplot2/R/scale-.r

Lines 597 to 598 in aaaec2f

axis <- if ("x" %in% self$aesthetics) "x" else "y"
check_transformation(x, new_x, self$scale_name, axis)

Which is fine for position scales, but generates these awkward messages for other types of scales.

I'm not sure how safe this is, but it might be possible to replace the first line in the chunk above by

axis <- self$aesthetics[1]

At least, that is how ggplot2:::guide_train.axis() retrieves the aesthetic from scales if not explicitly given as argument.

In a similar vein, ggplot2:::check_transformation() only considers the names of position scales for generating the message itself:

ggplot2/R/scale-.r

Lines 1185 to 1191 in aaaec2f

type <- if (name == "position_b") {
"binned"
} else if (name == "position_c") {
"continuous"
} else {
"discrete"
}

It think an alternative might be to consider to use the inherits(scale, "ScaleContinuous") pattern above (which should be TRUE for both position and non-position continuous scales, but not for binned or discrete scales), but the scale itself is not passed as an argument.

@clauswilke
Copy link
Member

In general, you have to be prepared for self$aesthetics to be a vector of multiple aesthetics that you all have to consider. So any code of the form self$aesthetics[1] makes me uncomfortable. There may be places where it's fine, but it can also go spectacularly wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
messages requests for improvements to error, warning, or feedback messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants