Allow functional limits in continuous scales#2334
Allow functional limits in continuous scales#2334thomasp85 merged 5 commits intotidyverse:masterfrom
Conversation
hadley
left a comment
There was a problem hiding this comment.
This is a small change and it seems useful, so lets do it! A few suggestions in the comments.
Also needs one or two unit tests. Can you please have a look at the existing tests and think about where they should go?
R/scale-.r
Outdated
| if (self$is_empty()) return(c(0, 1)) | ||
|
|
||
| if (!is.null(self$limits)) { | ||
| if (is.function(self$limits)) { |
There was a problem hiding this comment.
Since we're adding another condition here, can you please reframe the if statement along these lines:
if (is.null()) {
} else if (is.function()) {
} else if (is.numeric()) {
} else {
stop("Informative error message", call. = FALSE)
}There was a problem hiding this comment.
I did rework this conditional to be more intuitive but since get_limits() is called by both discrete and continuous scales and can be passed character, numeric, POSIXt and now function classes, I ultimately did not add an explicit is.numeric conditional nor an error message. Happy to update further if necessary.
|
Hey @econandrew if you're still interested in this PR, I'm around (:wave: summer intern!) and can help you get this over the finish line if you'd like. Let me know if I can help in anyway. |
|
@dpseidel would you consider finishing this off? |
|
Yep happy to. |
Merge remote-tracking branch 'upstream/master' into allow-functional-limits # Conflicts: # R/scale-.r
|
The diff seems to include spurious changes which makes it hard to review. |
|
Sorry, yes, I got a little nit picky on the NEWS doc, clearly that should be in a different PR. All cleaned up now. |
thomasp85
left a comment
There was a problem hiding this comment.
LGTM - thanks for finishing it of Dana
|
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/ |
An implementation of the idea in #2307, for consideration.
Motivation is to allow programmatic, data-agnostic control of scale limits, taking advantage of rather than having to replicate ggplot's aesthetic calculations. For example, suppose you want limits to always be a multiple of 50: