Skip to content

all.equal.data.table gets ignore.indices argument #6136

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.

Sign up for GitHub

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

Open
mb706 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

all.equal.data.table gets ignore.indices argument #6136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@

13. `dcast`gains `value.var.in.dots`, `value.var.in.LHSdots` and `value.var.in.RHSdots` arguments, [#5824](https://github.com/Rdatatable/data.table/issues/5824). This allows the `value.var` variable(s) in `dcast` to be represented by `...` in the formula (if not otherwise mentioned). Thanks to @iago-pssjd for the report and PR.

14. `all.equal.data.table` gains `ignore.indices` argument, [#6134](https://github.com/Rdatatable/data.table/issues/6134).

## BUG FIXES

1. `unique()` returns a copy the case when `nrows(x) <= 1` instead of a mutable alias, [#5932](https://github.com/Rdatatable/data.table/pull/5932). This is consistent with existing `unique()` behavior when the input has no duplicates but more than one row. Thanks to @brookslogan for the report and @dshemetov for the fix.
Expand Down
27 changes: 15 additions & 12 deletions R/setops.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ fsetequal = function(x, y, all=TRUE) {

# all.equal ----

all.equal.data.table = function(target, current, trim.levels=TRUE, check.attributes=TRUE, ignore.col.order=FALSE, ignore.row.order=FALSE, tolerance=sqrt(.Machine$double.eps), ...) {
stopifnot(is.logical(trim.levels), is.logical(check.attributes), is.logical(ignore.col.order), is.logical(ignore.row.order), is.numeric(tolerance), is.data.table(target))
all.equal.data.table = function(target, current, trim.levels=TRUE, check.attributes=TRUE, ignore.indices=FALSE, ignore.col.order=FALSE, ignore.row.order=FALSE, tolerance=sqrt(.Machine$double.eps), ...) {
Copy link
Member

@ben-schwen ben-schwen May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we are adding a new argument in the middle? This might break downstream code.

Copy link
Author

@mb706 mb706 May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to group ignore.indices with check.attributes as they seem most related, as indices are attributes. I would say that users should always pass all.equal's optional arguments by name, since these are usually passed on recursively, which makes positional argument passing risky with all.equal beyond the first two. If you want me to, I can move ignore.indices to the end.

stopifnot(is.logical(trim.levels), is.logical(check.attributes), is.logical(ignore.indices), is.logical(ignore.col.order), is.logical(ignore.row.order), is.numeric(tolerance), is.data.table(target))

if (!is.data.table(current)) {
if (check.attributes) return(paste0('target is data.table, current is ', data.class(current)))
Expand Down Expand Up @@ -163,20 +163,23 @@ all.equal.data.table = function(target, current, trim.levels=TRUE, check.attribu
))
}
# check index
i1 = indices(target)
i2 = indices(current)
if (!identical(i1, i2)) {
return(gettextf(
"Datasets have different %s. 'target': %s. 'current': %s.",
"indices",
if(length(i1)) brackify(i1) else gettextf("has no index"),
if(length(i2)) brackify(i2) else gettextf("has no index")
))
if (!ignore.indices) {
i1 = indices(target)
i2 = indices(current)
if (!identical(i1, i2)) {
return(gettextf(
"Datasets have different %s. 'target': %s. 'current': %s.",
"indices",
if(length(i1)) brackify(i1) else gettextf("has no index"),
if(length(i2)) brackify(i2) else gettextf("has no index")
))
}
}

# Trim any extra row.names attributes that came from some inheritance
# Trim ".internal.selfref" as long as there is no `all.equal.externalptr` method
exclude.attrs = function(x, attrs = c("row.names",".internal.selfref")) x[!names(x) %chin% attrs]
# Trim "index" attribute, in case `ignore.indices` is set (if it is not, we already checked it).
exclude.attrs = function(x, attrs = c("row.names",".internal.selfref", "index")) x[!names(x) %chin% attrs]
a1 = exclude.attrs(attributes(target))
a2 = exclude.attrs(attributes(current))
if (length(a1) != length(a2)) return(sprintf("Datasets has different number of (non-excluded) attributes: target %s, current %s", length(a1), length(a2)))
Expand Down
2 changes: 2 additions & 0 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8684,6 +8684,7 @@ DT1 <- data.table(a = 1:4, b = letters[1:4])
DT2 <- data.table(a = 1:4, b = letters[1:4])
setindexv(DT1, "b")
test(1613.25, all.equal(DT1, DT2), "Datasets have different indices. 'target': [b]. 'current': has no index.")
test(1613.251, all.equal(DT1, DT2, ignore.indices = TRUE), TRUE)
test(1613.26, all.equal(DT1, DT2, check.attributes = FALSE), TRUE)
test(1613.27, all.equal(DT1, setindexv(DT2, "a")), "Datasets have different indices. 'target': [b]. 'current': [a].")
test(1613.28, all.equal(DT1, setindexv(DT2, "b")), "Datasets have different indices. 'target': [b]. 'current': [a, b].")
Expand All @@ -8693,6 +8694,7 @@ DT1 <- data.table(a = 1:4, b = letters[1:4])
DT2 <- data.table(a = 1:4, b = letters[1:4])
setattr(DT1, "custom", 1L)
test(1613.30, all.equal(DT1, DT2), "Datasets has different number of (non-excluded) attributes: target 3, current 2")
test(1613.301, all.equal(DT1, DT2, ignore.indices = TRUE), "Datasets has different number of (non-excluded) attributes: target 3, current 2")
test(1613.31, all.equal(DT1, DT2, check.attributes = FALSE), TRUE)
setattr(DT2, "custom2", 2L)
test(1613.32, all.equal(DT1, DT2), "Datasets has attributes with different names: [custom, custom2]")
Expand Down
8 changes: 6 additions & 2 deletions man/all.equal.data.table.Rd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

\usage{
\method{all.equal}{data.table}(target, current, trim.levels=TRUE, check.attributes=TRUE,
ignore.col.order=FALSE, ignore.row.order=FALSE, tolerance=sqrt(.Machine$double.eps),
\dots)
ignore.indices=FALSE, ignore.col.order=FALSE, ignore.row.order=FALSE,
tolerance=sqrt(.Machine$double.eps), \dots)
}

\arguments{
Expand All @@ -27,6 +27,10 @@
A logical indicating whether or not to check attributes, will apply not only to data.table but also attributes of the columns. It will skip \code{c("row.names",".internal.selfref")} data.table attributes.
}

\item{ignore.indices}{
A logical indicating whether or not to ignore if tables differ by indices. Since \code{data.table} indices are attributes, \code{ignore.indices} is implied when \code{check.attributes} is FALSE.
}

\item{ignore.col.order}{
A logical indicating whether or not to ignore columns order in \code{data.table}.
}
Expand Down