Repository navigation
srcref diff is shown for S7 classes if there are other differences #217
Description
# as expected
library(S7)
#> Warning: package 'S7' was built under R version 4.4.1
fun <- function() {}
waldo::compare(fun, structure(fun, srcref = 1))
#> ✔ No differences
waldo::compare(fun, structure(fun, srcref = 1, foo = 2))
#> `attr(old, 'foo')` is absent
#> `attr(new, 'foo')` is a double vector (2)
waldo::compare(S7_object, structure(S7_object, srcref = 1))
#> ✔ No differences
# unexpected, shouldn't show srcref diff
waldo::compare(S7_object, structure(S7_object, srcref = 1, foo = 2))
#> `attr(old, 'foo')` is absent
#> `attr(new, 'foo')` is a double vector (2)
#>
#> `attr(old, 'srcref')` is absent
#> `attr(new, 'srcref')` is a double vector (1)Created on 2025-06-03 with reprex v2.1.0
if there is no difference apart from srcref in compare_structure() we don't go further than the is_identical() check but if we do we enter the S7 handling part and attrs() doesn't dismiss the "srcref" from the inspection.
Happy to do a PR if it helps.
Activity
hadley commentedon Jul 7, 2025
Can you give me a bit more context here? waldo currently only removes srcrefs from functions (which I think makes sense).
moodymudskipper commentedon Jul 8, 2025
It doesn't remove srcref from all functions however,
S7_Objectis a function and its srcref is not removed inwaldo::compare(S7_object, structure(S7_object, srcref = 1, foo = 2)).Maybe you intended
is.object(x)to testtypeof(x) == "object"?is.object()tests for the OBJECT bit that is always set for classed objects, so in fact your conditionis.object(x) && inherits(x, "S7_object")is redundant (is.object(structure(NA, class = "S7_object"))isTRUE).In
compare_structure()we find in order:is_identical()check, that checks if closures (S7 or not) are identical when we remove the srcref, causingwaldo::compare(S7_object, structure(S7_object, srcref = 1))to show no diffis.object(x) && inherits(x, "S7_object")check, that does not remove the srcref, causingwaldo::compare(S7_object, structure(S7_object, srcref = 1, foo = 2))to show shows both the foo difference but also the srcref diff despiteS7_objectbeing a closure.is.closure()check that does remove the srcref, but not all closures get thereIMO srcrefs should be removed from all closures by default, S7 or not, and if we don't do it I believe
is_identical()should have an exception.moodymudskipper commentedon Jul 24, 2025
I was wrong above to assume
is.object()was redundant,inherits()fails on CHARSXP so it was indeed necessary.