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

expect_s3_class() should handle not-S3 classes [FR] #1321

Closed
DanChaltiel opened this issue Feb 7, 2021 · 1 comment
Closed

expect_s3_class() should handle not-S3 classes [FR] #1321

DanChaltiel opened this issue Feb 7, 2021 · 1 comment
Labels
expectation 🙀 feature a feature request or enhancement

Comments

@DanChaltiel
Copy link

DanChaltiel commented Feb 7, 2021

In some cases, you want to remove a class from an object.

Then, if you removed the last S3 class, you should obviously expect an object without any S3 class.

However, expect_s3_class() will not allow testing if an object is no longer an S3 class.

Here is a simple example with labels, using expss:::unlab.default():

library(expss)
library(testthat)
x=iris
class(x$Sepal.Length)
#> [1] "numeric"
var_lab(x$Sepal.Length) = "Sepal length"
var_lab(x$Sepal.Length)
#> [1] "Sepal length"
class(x$Sepal.Length)
#> [1] "labelled" "numeric"
expect_s3_class(x$Sepal.Length, "labelled/numeric")
#> Error: x$Sepal.Length inherits from `labelled/numeric` not `labelled/numeric`.
x$Sepal.Length = unlab(x$Sepal.Length)
class(x$Sepal.Length)
#> [1] "numeric"
expect_s3_class(x$Sepal.Length, "numeric")
#> Error: x$Sepal.Length is not an S3 object
expect_s3_class(x$Sepal.Length, NULL)
#> Error in expect_s3_class(x$Sepal.Length, NULL): is.character(class) is not TRUE

Created on 2021-02-07 by the reprex package (v1.0.0)

Here are some potential syntaxes I could think of:

  • expect_s3_class(x$Sepal.Length, NULL)
  • expect_s3_class(x$Sepal.Length, isS3=FALSE)
  • expect_not_s3(x$Sepal.Length)

IMHO, the first one feels the most logical and should be the easiest to implement.

EDIT:

As you can see, I've been confused by the error message "Error: x$Sepal.Length inherits from labelled/numeric not labelled/numeric.". See #1322.

@hadley
Copy link
Member

hadley commented Feb 7, 2021

I think expect_s3_class(x, NA) would be best because it implies that the S3 class is "missing", and it matches the syntax of expect_error() and friends.

@hadley hadley added expectation 🙀 feature a feature request or enhancement labels Feb 7, 2021
@hadley hadley closed this as completed in ecfa8a6 Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expectation 🙀 feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants