Fix Travis failures on r-devel #3163
Conversation
|
I believe |
|
I see. One of the two |
R/guide-colorbar.r
Outdated
| @@ -244,7 +244,7 @@ guide_geom.colorbar <- function(guide, layers, default_mapping) { | |||
| guide_layers <- lapply(layers, function(layer) { | |||
| matched <- matched_aes(layer, guide, default_mapping) | |||
|
|
|||
| if (length(matched) && ((is.na(layer$show.legend) || layer$show.legend))) { | |||
| if (length(matched) && (isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend))) { | |||
There was a problem hiding this comment.
Should length(matched) get an explicit comparison, like length(matched) > 0 (I assume that's what is meant)?
R/scale-.r
Outdated
| @@ -563,7 +563,7 @@ continuous_scale <- function(aesthetics, scale_name, palette, name = waiver(), | |||
|
|
|||
| position <- match.arg(position, c("left", "right", "top", "bottom")) | |||
|
|
|||
| if (is.null(breaks) && !is_position_aes(aesthetics) && guide != "none") { | |||
| if (is.null(breaks) && all(is_position_aes(aesthetics)) && identical(guide, "none")) { | |||
There was a problem hiding this comment.
I'm confused here. Doesn't this invert the logic for the last two checks? Wouldn't they need ! in front?
There was a problem hiding this comment.
Opps..., sorry, I'm also confused because the previous logic seems unclear to me about its intension. Let me think. (I believe all(is_position_aes(aesthetics)) doesn't need !; if there are only positional aesthetics, we don't need guides, right?)
R/scale-.r
Outdated
| @@ -634,7 +634,7 @@ discrete_scale <- function(aesthetics, scale_name, palette, name = waiver(), | |||
|
|
|||
| position <- match.arg(position, c("left", "right", "top", "bottom")) | |||
|
|
|||
| if (is.null(breaks) && !is_position_aes(aesthetics) && guide != "none") { | |||
| if (is.null(breaks) && all(is_position_aes(aesthetics)) && identical(guide, "none")) { | |||
There was a problem hiding this comment.
Same as previous. Inverted logic?
R/utilities.r
Outdated
| @@ -475,3 +475,6 @@ parse_safe <- function(text) { | |||
| } | |||
| out | |||
| } | |||
|
|
|||
| # For older version of R | |||
| isFALSE <- function(x) is.logical(x) && length(x) == 1L && !is.na(x) && !x | |||
There was a problem hiding this comment.
Should this be in backports.R and have a version check?
There was a problem hiding this comment.
Sure, it seems the right place.
|
Thanks for the prompt review! |
* Wrap multiple logicals with isTRUE(), isFALSE(), all() or any() to ensure the errors related to R_CHECK_LENGTH_1_LOGIC2 * Backport isFALSE()
* Wrap multiple logicals with isTRUE(), isFALSE(), all() or any() to ensure the errors related to R_CHECK_LENGTH_1_LOGIC2 * Backport isFALSE()
|
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
It seems the failure on r-devel is related to
R_CHECK_LENGTH_1_LOGIC2, an option to raise error when multiple logicals are passed to||or&&.I'm not sure the next version of R will be released with this strict behaviour, but it's a nice chance to fix inappropriate if conditions.