pik-piam / magclass

R package | Data Class and Tools for Handling Spatial-Temporal Data
GNU Lesser General Public License v3.0
4 stars 24 forks source link

waldo::compare_proxy method for magclass objects #112

Closed johanneskoch94 closed 2 years ago

johanneskoch94 commented 2 years ago

Hi, I think it would be super cool, to have a waldo::compare method for magclass objects, so that the testthat::expect functions work nicely. I think it could start out simply, by ignoring the comment$date entry, and work from there, maybe with options to ignore all comments, or not. What do you think? (I for now tend to do the following:

x <- calcOutput("GDPpc", extension2150 = "none", naming = "scenario", aggregate = FALSE)
y <- calcOutput("GDP", extension2150 = "none", naming = "scenario", aggregate = FALSE) /
    calcOutput("Population", extension2150 = "none", naming = "scenario", aggregate = FALSE)
# Remove comments before comparing
comment(x) <- NULL
comment(y) <- NULL
expect_equal(x, y)

) (see https://waldo.r-lib.org/ and https://waldo.r-lib.org/reference/compare_proxy.html)

johanneskoch94 commented 2 years ago

Oh, and I've tried to use the "ignore_attr" argument, but I can't get it to work. That's why I propose a method, but maybe I just didn't do it correctly.

tscheypidi commented 2 years ago

Interesting suggestion. I did some testing and in my case it looked like a compare_proxy.magpie is changing the behavior of waldo::compare but not testhat::expect, but I had success via introducing a compare.magclass method.

However, after building the prototype I realized that I cannot rule out that the content of the comment actually might play a role in some tests. So removing the comment in general does not seem a good idea to me anymore. So, I suggest that you stick to the procedure you are currently using in which you explicitly remove the comment.

johanneskoch94 commented 2 years ago

Okok. Indeed, I haven't figured out how to pass an additional argument to compare_proxy, to control exactly what is being ignored or not. It may not be possible.

I'll just leave this here for future reference and others: When creating tests in a package that compare magpie objects, adding this method to your package makes testthat::expect_equal(magpie1, magpie2) ignore the "origin" and "creation date" comments.

compare_proxy.magpie <- function(x, path = "x") {
  if (any(grepl("^ (origin:|creation date:)", comment(x)))) {
    comment(x) <- comment(x)[!grepl("^ (origin:|creation date:)", comment(x))]
  }
  list(object = x, path = path)
}