Improve handling of abstract S3 classes#692
Conversation
* No longer error when constructing a class that inherits from an abstract S3 class * Errors when upcasting to an abstract S3 class * New test to confirm that `super()` works for an abstract S3 class. No news bullet for the motivating issue since this was a bug introduced in the dev version. Fixes #686
| # registered without a constructor (e.g. a marker class like "gg" or "POSIXt"). | ||
| class_is_abstract <- function(class) { | ||
| (is_class(class) && class@abstract) || | ||
| (is_S3_class(class) && class$abstract) |
There was a problem hiding this comment.
Should we use isTRUE(class$abstract) here, in case the class is missing the $abstract field? This could happen if people are using the new S7 with S7 objects that were constructed with older S7 (i.e., built packages or serialized class defs generated with current release S7).
There was a problem hiding this comment.
I think that's pretty low likelihood given the relative rarity of abstract S3 classes. Although I should double check that this fixes the ggplot2 issue.
There was a problem hiding this comment.
isTRUE() doesn't actually help because we need to work with S3 classes created by older S7. And while these are obviously rare in the scheme of things they do occur in ggplot2, hence this issue.
I've added a fallback and verified that this now does actually fix the ggplot2 loading issue.
super()works for an abstract S3 class.No news bullet for the motivating issue since this was a bug introduced in the dev version.
Fixes #686