-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Forward compatibility: is_*()
functions
#6388
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
Forward compatibility: is_*()
functions
#6388
Conversation
is_element <- function(x, type = "any") { | ||
switch( | ||
type %||% "any", | ||
any = inherits(x, "element"), | ||
rect = inherits(x, "element_rect"), | ||
line = inherits(x, "element_line"), | ||
text = inherits(x, "element_text"), | ||
blank = inherits(x, "element_blank"), | ||
# TODO: ideally we accept more elements from extensions. We need to | ||
# consider how this will work with S7 classes, where ggplot2 doesn't know | ||
# about the extension's class objects. | ||
FALSE | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thomas, I'd like to draw your eye to this bit with regard to the comment here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to support anything but our own classes here tbh
is_element <- function(x, type = "any") { | ||
switch( | ||
type %||% "any", | ||
any = inherits(x, "element"), | ||
rect = inherits(x, "element_rect"), | ||
line = inherits(x, "element_line"), | ||
text = inherits(x, "element_text"), | ||
blank = inherits(x, "element_blank"), | ||
# TODO: ideally we accept more elements from extensions. We need to | ||
# consider how this will work with S7 classes, where ggplot2 doesn't know | ||
# about the extension's class objects. | ||
FALSE | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to support anything but our own classes here tbh
This PR aims to improve forwards compatibility.
Briefly, it re-implements parts of #6022 specifically the test functions. We deviate from that PR through deprecating dot.case functions and introducing snake_case function names are used instead.
Notably,
is_element(x, type = ....)
allows one to test for specific subtypes of elements as well.