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

NA values break plot.errors #35

Closed
richardsc opened this issue Dec 6, 2019 · 5 comments
Closed

NA values break plot.errors #35

richardsc opened this issue Dec 6, 2019 · 5 comments
Labels

Comments

@richardsc
Copy link

Just discovered this package (after thinking about writing my own) -- it's great! Thanks!

I noticed that xlim and ylim are hard-coded in the NextMethod() call for plot.errors() to be range(x) and range(y).

One problem with this is that it is impossible to control the axis limits of the plot (say, to focus on a region or reverse an axes).

Another issue is that if x or y have NAs they cause plot.errors() to fail:

library(errors)
x <- 1:100
y <- rnorm(x)
y[20] <- NA
errors(x) <- 0
errors(y) <- rnorm(y, sd=0.25)
plot(x, y)
Error in plot.window(...) : need finite 'ylim' values

I don't see why plot.errors() isn't simply written as:

plot.errors <- function(x, y, ...) {
  if (missing(y)) {
    y <- x
    x <- seq_along(x)
  }

  NextMethod()

  if (inherits(x, "errors"))
    graphics::segments(errors_min(x), y, errors_max(x), y)

  if (inherits(y, "errors"))
    graphics::segments(x, errors_min(y), x, errors_max(y))
}

Note that using the na.rm=TRUE argument in range() doesn't help, as it still returns c(NA, NA) for range(y). I assume that this is due to the interaction the base range() function with the errors-class object and it doesn't know what to do with it?

@dankelley
Copy link

I also have come to the errors package only in the past few weeks (and I had actually written a similar, but much less useful package, a few days ago).

This range problem is in R/summary.R line 32, viz.

    "range" = c(min(...), max(...))

which I think ought to read

    "range" = c(min(..., na.rm=na.rm), max(..., na.rm=na.rm))

@dankelley
Copy link

Oh, and also the which.min and which.max calls above line 32 could also use adjustment.

@Enchufa2
Copy link
Member

Enchufa2 commented Dec 6, 2019

Thanks for your reports.

@dankelley You are right about range, it should be fixed in 8f95673. which.min and which.max have no na.rm argument though.

@richardsc

I don't see why plot.errors() isn't simply written as [...]

The problem is that we don't want error bars to be cut out of the plotting range by default, but I agree that a user-provided xlim or ylim should be respected. So it's a little more complicated than that. I'm working on it.

@Enchufa2
Copy link
Member

Enchufa2 commented Dec 6, 2019

In fact, it was a incorrect implementation of min, max and range. Rewritten in 61b004c.

@Enchufa2 Enchufa2 added the bug label Dec 6, 2019
@Enchufa2
Copy link
Member

Enchufa2 commented Dec 6, 2019

plot fixed in a3a6f0f, I think. Could you please test the current master branch, @richardsc?

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

No branches or pull requests

3 participants