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

easy_exact_limits #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

easy_exact_limits #13

wants to merge 2 commits into from

Conversation

jonocarroll
Copy link
Owner

easy_exact_limits
easy_exact_xlim
easy_exact_ylim
teach argument implemented
no tests yet
shortcuts vignette updated

Is this what you had in mind in #1 @AliciaSchep? Happy to add in more functionality if any is needed.

easy_exact_xlim
easy_exact_ylim
teach argument implemented
no tests yet
shortcuts vignette updated
@AliciaSchep
Copy link
Collaborator

AliciaSchep commented Nov 16, 2017

Not quite what I was thinking, but close. I think the behavior of defaulting to removing the extra space on the lower limit maps well to the motivating use in the issue I created, but is maybe a bit surprising for the function name & current documentation.

I was thinking something more like:

easy_exact_ylim2 <- function(..., teach = FALSE){
  
  lims <- c(...)
  stopifnot(length(lims) == 2)
  
  if (!any(is.na(lims)) && lims[1] > lims[2]) {
    trans <- "reverse"
  }
  else {
    trans <- "identity"
  }
  
  default_expand <- 0.1
  
  expand <- rep(0,4)
  if (is.na(lims[1])){
    expand[1] <- default_expand
  }
  if (is.na(lims[2])){
    expand[2] <- default_expand
  }
  
  # Teach goes here
  
  return(scale_y_continuous(limits = lims, expand = expand, trans = trans))
  
}

Which you would then call

ggplot(mtcars) +
      geom_bar(aes(x = factor(cyl))) +
        easy_exact_ylim2(0,NA)

To get the bar chart at zero effect.

It would basically be just like xlim except for also setting expand to zero for the non-NA value.

To make it easier to just give max or min, I was thinking of making it easy_exact_ylim(..., max = NA, min = NA, teach = FALSE) so you could optionally call easy_exact_ylim(min = 0) but you could also call it similarly to xlim. I think having the functions being a pretty easy drop-in for ylim and xlim would be nice

I also think having the option to alter the expand argument actually takes away some of the usefulness of this function -- part of what is annoying to remember about setting exact limits is that you have to remember what the positions in the expand vector stand for (I literally look this up every time). Having it as an option here would again make you have to potentially figure out that vector, while having the value be zero if the limit is not NA would fit the function name (of making limits exact) and take the burden of dealing with that vector away from the user

@jonocarroll
Copy link
Owner Author

That all sounds good. I'll iterate and update.

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

Successfully merging this pull request may close these issues.

2 participants