Skip to content

Fix Travis failures on r-devel #3163

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Feb 25, 2019
Merged

Fix Travis failures on r-devel #3163

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e9b8ff7
Ensure if conditions are length 1
yutannihilation Feb 24, 2019
7f1c39b
Fix bugs
yutannihilation Feb 24, 2019
1b5f10b
Fix a misused &
yutannihilation Feb 24, 2019
a53e2f9
Wrap with isTRUE() and isFALSE()
yutannihilation Feb 24, 2019
5b740fe
Wrap more with isTRUE() and isFALSE()
yutannihilation Feb 24, 2019
425bafc
Fix misplaced parenthesis
yutannihilation Feb 24, 2019
43b0e61
Change isFALSE() to !isTRUE()
yutannihilation Feb 25, 2019
387ad72
Explicitly check length(matched)
yutannihilation Feb 25, 2019
0db9a69
Move isFALSE() to backports
yutannihilation Feb 25, 2019
800b29e
Fix break = NULL for non-positional scales
yutannihilation Feb 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion R/guide-colorbar.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Copy link
Member

Choose a reason for hiding this comment

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

Should length(matched) get an explicit comparison, like length(matched) > 0 (I assume that's what is meant)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, thanks.

layer
} else {
# This layer does not use this guide
Expand Down
4 changes: 2 additions & 2 deletions R/guide-legend.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ guide_geom.legend <- function(guide, layers, default_mapping) {
include <- is.na(layer$show.legend[matched]) ||
layer$show.legend[matched]
} else {
include <- is.na(layer$show.legend) || layer$show.legend
include <- isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend)
}

if (include) {
Expand All @@ -271,7 +271,7 @@ guide_geom.legend <- function(guide, layers, default_mapping) {
}
} else {
# This layer does not contribute to the legend
if (is.na(layer$show.legend) || !layer$show.legend) {
if (isTRUE(is.na(layer$show.legend)) || !isTRUE(layer$show.legend)) {
# Default is to exclude it
return(NULL)
} else {
Expand Down
4 changes: 2 additions & 2 deletions R/guides-.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ guides_train <- function(scales, theme, guides, labels) {
# this should be changed to testing guide == "none"
# scale$legend is backward compatibility
# if guides(XXX=FALSE), then scale_ZZZ(guides=XXX) is discarded.
if (guide == "none" || (is.logical(guide) && !guide)) next
if (identical(guide, "none") || isFALSE(guide)) next

# check the validity of guide.
# if guide is character, then find the guide object
guide <- validate_guide(guide)

# check the consistency of the guide and scale.
if (guide$available_aes != "any" && !scale$aesthetics %in% guide$available_aes)
if (!identical(guide$available_aes, "any") && !any(scale$aesthetics %in% guide$available_aes))
stop("Guide '", guide$name, "' cannot be used for '", scale$aesthetics, "'.")

guide$title <- scale$make_title(guide$title %|W|% scale$name %|W|% labels[[output]])
Expand Down
2 changes: 1 addition & 1 deletion R/position-stack.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ PositionFill <- ggproto("PositionFill", PositionStack,

stack_var <- function(data) {
if (!is.null(data$ymax)) {
if (any(data$ymin != 0 && data$ymax != 0, na.rm = TRUE)) {
if (any(data$ymin != 0 & data$ymax != 0, na.rm = TRUE)) {
warning("Stacking not well defined when not anchored on the axis", call. = FALSE)
}
"ymax"
Expand Down
4 changes: 2 additions & 2 deletions R/scale-.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused here. Doesn't this invert the logic for the last two checks? Wouldn't they need ! in front?

Copy link
Member Author

Choose a reason for hiding this comment

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

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?)

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I got it. #581 and #579 are the context; breaks = NULL can be used for removing the guide of a non-positional scales.

guide <- "none"
}

Expand Down Expand Up @@ -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")) {
Copy link
Member

Choose a reason for hiding this comment

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

Same as previous. Inverted logic?

guide <- "none"
}

Expand Down
3 changes: 3 additions & 0 deletions R/utilities.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Should this be in backports.R and have a version check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, it seems the right place.