Skip to content

Attempt S4 dispatch for intersect() and union(), maybe - alternative #307

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

Closed
hcirellu wants to merge 2 commits into from
Closed

Attempt S4 dispatch for intersect() and union(), maybe - alternative #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
974f2c7
using S4 setMethod to dispatch S4 classes on intersect.integer64
hcirellu Mar 19, 2026
0858b01
add documentation
hcirellu Mar 19, 2026
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
15 changes: 10 additions & 5 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,10 @@ S3method(hashtab,cache_integer64)
S3method(hashuni,cache_integer64)
S3method(hashupo,cache_integer64)
S3method(identical,integer64)
S3method(intersect,integer64)
S3method(is.double,default)
S3method(is.double,integer64)
S3method(is.element,integer64)
S3method(is.finite,integer64)
S3method(is.infinite,integer64)
S3method(is.na,integer64)
Expand Down Expand Up @@ -155,6 +157,8 @@ S3method(rowSums,default)
S3method(rowSums,integer64)
S3method(scale,integer64)
S3method(seq,integer64)
S3method(setdiff,integer64)
S3method(setequal,integer64)
S3method(shellorder,integer64)
S3method(shellsort,integer64)
S3method(shellsortorder,integer64)
Expand Down Expand Up @@ -182,6 +186,7 @@ S3method(table,default)
S3method(table,integer64)
S3method(tiepos,integer64)
S3method(trunc,integer64)
S3method(union,integer64)
S3method(unipos,integer64)
S3method(unique,integer64)
export("%in%")
Expand Down Expand Up @@ -228,9 +233,7 @@ export(hashuni)
export(hashupo)
export(identical.integer64)
export(integer64)
export(intersect)
export(is.double)
export(is.element)
export(is.integer64)
export(is.na.integer64)
export(jamcache)
Expand Down Expand Up @@ -271,8 +274,6 @@ export(rowSums)
export(runif64)
export(seq.integer64)
export(setcache)
export(setdiff)
export(setequal)
export(sortcache)
export(sortfin)
export(sortnut)
Expand All @@ -292,9 +293,13 @@ export(str.integer64)
export(sum.integer64)
export(table)
export(tiepos)
export(union)
export(unipos)
export(unique.integer64)
exportMethods(intersect)
exportMethods(is.element)
exportMethods(setdiff)
exportMethods(setequal)
exportMethods(union)
if (getRversion() >= "4.2.0") S3method(mtfrm,integer64)
if (getRversion() >= "4.3.0") S3method(chooseOpsMethod,integer64)
importFrom(bit,clone)
Expand Down
56 changes: 51 additions & 5 deletions R/setops64.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
#'
#' @export
#' @rdname sets
union = function(x, y) {
#' @name sets
union.integer64 = function(x, y) {
if (!(is.integer64(x) || is.integer64(y)))
return(base::union(x, y))

Expand All @@ -55,10 +56,19 @@ union = function(x, y) {

unique(c(x, y))
}
#' @exportMethod union
#' @rdname sets
setMethod("union", signature(x="ANY", y="ANY"), union.integer64)
Copy link
Copy Markdown
Collaborator

@MichaelChirico MichaelChirico Mar 19, 2026

Choose a reason for hiding this comment

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

In {nanotime}, they leave this unspecified and therefore it becomes base::intersect().

Overall I might expect we remove this from methods:

if (!(is.integer64(x) || is.integer64(y))) return(base::...)

In favor of S4 dispatch handling that part. Then we only need to define for signature(x="integer64", y="ANY") and signature(x="ANY", y="integer64").

Also, is there any residual benefit of the S3 method once we add the S4 dispatch?

#' @rdname sets
setMethod("union", signature(x="integer64", y="ANY"), union.integer64)
#' @rdname sets
setMethod("union", signature(x="ANY", y="integer64"), union.integer64)
#' @rdname sets
setMethod("union", signature(x="integer64", y="integer64"), union.integer64)

#' @export
#' @rdname sets
intersect = function(x, y) {
intersect.integer64 = function(x, y) {
if (is.null(x) || is.null(y)) return(NULL)
if (!(is.integer64(x) || is.integer64(y)))
return(base::intersect(x, y))
Expand All @@ -83,10 +93,19 @@ intersect = function(x, y) {

x[match(x, y, 0L) > 0L]
}
#' @exportMethod intersect
#' @rdname sets
setMethod("intersect", signature(x="ANY", y="ANY"), intersect.integer64)
#' @rdname sets
setMethod("intersect", signature(x="integer64", y="ANY"), intersect.integer64)
#' @rdname sets
setMethod("intersect", signature(x="ANY", y="integer64"), intersect.integer64)
#' @rdname sets
setMethod("intersect", signature(x="integer64", y="integer64"), intersect.integer64)

#' @export
#' @rdname sets
setequal = function(x, y) {
setequal.integer64 = function(x, y) {
if (!(is.integer64(x) || is.integer64(y)))
return(base::setequal(x, y))

Expand All @@ -105,10 +124,19 @@ setequal = function(x, y) {

!anyNA(match(x, y))
}
#' @exportMethod setequal
#' @rdname sets
setMethod("setequal", signature(x="ANY", y="ANY"), setequal.integer64)
#' @rdname sets
setMethod("setequal", signature(x="integer64", y="ANY"), setequal.integer64)
#' @rdname sets
setMethod("setequal", signature(x="ANY", y="integer64"), setequal.integer64)
#' @rdname sets
setMethod("setequal", signature(x="integer64", y="integer64"), setequal.integer64)

#' @export
#' @rdname sets
setdiff = function(x, y) {
setdiff.integer64 = function(x, y) {
if (!(is.integer64(x) || is.integer64(y)))
return(base::setdiff(x, y))

Expand All @@ -134,10 +162,19 @@ setdiff = function(x, y) {

x[match(x_match, y, 0L) == 0L]
}
#' @exportMethod setdiff
#' @rdname sets
setMethod("setdiff", signature(x="ANY", y="ANY"), setdiff.integer64)
#' @rdname sets
setMethod("setdiff", signature(x="integer64", y="ANY"), setdiff.integer64)
#' @rdname sets
setMethod("setdiff", signature(x="ANY", y="integer64"), setdiff.integer64)
#' @rdname sets
setMethod("setdiff", signature(x="integer64", y="integer64"), setdiff.integer64)

#' @export
#' @rdname sets
is.element = function(el, set) {
is.element.integer64 = function(el, set) {
if (!(is.integer64(el) || is.integer64(set)))
return(base::is.element(el, set))

Expand All @@ -160,3 +197,12 @@ is.element = function(el, set) {

match(el, set, 0L) > 0L
}
#' @exportMethod is.element
#' @rdname sets
setMethod("is.element", signature(el="ANY", set="ANY"), is.element.integer64)
#' @rdname sets
setMethod("is.element", signature(el="integer64", set="ANY"), is.element.integer64)
#' @rdname sets
setMethod("is.element", signature(el="ANY", set="integer64"), is.element.integer64)
#' @rdname sets
setMethod("is.element", signature(el="integer64", set="integer64"), is.element.integer64)
83 changes: 72 additions & 11 deletions man/sets.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 76 additions & 0 deletions tests/testthat/test-setops64.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,79 @@ test_that("is.element works (additional cases)", {
expect_true(is.element(NA_integer64_, NA))

})

test_that("S4 dispatch still happens for classes extending integer64 (#301)", {
methods::setClass("TestS4", representation(data="integer"))

delete_intersect_generic = !methods::isGeneric("intersect")
suppressMessages(methods::setGeneric("intersect"))
delete_union_generic = !methods::isGeneric("union")
suppressMessages(methods::setGeneric("union"))
delete_setdiff_generic = !methods::isGeneric("setdiff")
suppressMessages(methods::setGeneric("setdiff"))

methods::setMethod(
"intersect",
signature=c("TestS4", "integer64"),
function(x, y) "Successfully routed to S4 method A!"
)
methods::setMethod(
"intersect",
signature=c("TestS4", "TestS4"),
function(x, y) "Successfully routed to S4 method B!"
)
methods::setMethod(
"union",
signature=c("TestS4", "integer64"),
function(x, y) "Successfully routed to S4 method A!"
)
methods::setMethod(
"union",
signature=c("TestS4", "TestS4"),
function(x, y) "Successfully routed to S4 method B!"
)
methods::setMethod(
"setdiff",
signature=c("TestS4", "integer64"),
function(x, y) "Successfully routed to S4 method A!"
)
methods::setMethod(
"setdiff",
signature=c("TestS4", "TestS4"),
function(x, y) "Successfully routed to S4 method B!"
)
withr::defer({
methods::removeMethod("setdiff", signature=c("TestS4", "integer64"))
methods::removeMethod("union", signature=c("TestS4", "integer64"))
methods::removeMethod("intersect", signature=c("TestS4", "integer64"))
methods::removeClass("TestS4")
if (delete_setdiff_generic) methods::removeGeneric("setdiff")
if (delete_union_generic) methods::removeGeneric("union")
if (delete_intersect_generic) methods::removeGeneric("intersect")
})

# Instantiate test objects
x = methods::new("TestS4", data = 1L)
y = as.integer64(2L)

expect_identical(intersect(x, y), "Successfully routed to S4 method A!")
expect_identical(union(x, y), "Successfully routed to S4 method A!")
expect_identical(setdiff(x, y), "Successfully routed to S4 method A!")
# NB: nanoival class is "complex64" -- it kludges complex to be a pair
# of integer64 vectors, but there is no complex64 class, so it just
# shows up on the inheritance chain as 'complex' --> need to ensure
# S4 gets invoked when possible even if the inputs don't directly test
# as being is("integer64").
expect_identical(intersect(x, x), "Successfully routed to S4 method B!")
expect_identical(union(x, x), "Successfully routed to S4 method B!")
expect_identical(setdiff(x, x), "Successfully routed to S4 method B!")
})

test_that("S3 dispatch still happens for classes extending integer64 (#298)", {
x = 1:2
y = as.integer64(1L)
class(y) = c('foo', 'integer64')
expect_identical(intersect(x, y), as.integer64(1L))
expect_identical(union(x, y), as.integer64(1:2))
expect_identical(setdiff(x, y), 2L)
})
Loading