Attempt S4 dispatch for intersect() and union(), maybe by MichaelChirico · Pull Request #305 · r-lib/bit64

MichaelChirico · 2026-03-19T06:35:54Z

Closes #301. The rough idea is something Gemini pulled out of its hat as a way to avoid incurring anything too crufty/any dependency on {nanotime}.

With this (and #306), {nanotime} suite finally passes with devel {bit64}.

LMK if this makes sense to you.

hcirellu · 2026-03-19T15:15:54Z

I have a different approach, which uses the regular S4 dispatch. I put it into a separate PR #307, to better compare the two.

MichaelChirico · 2026-03-19T16:28:11Z

What are your own thoughts on this approach? I see the main advantages as (1) simpler to maintain and (2) doesn't require us to become more "S4" in our package design generally.

hcirellu · 2026-03-19T16:44:08Z

I agree with the advantages to be closer to a S3 package. I understand this S4 inclusion as a helper for a certain amount of time for other S4 packages, to adjust their code to use a more R consistent bit64 package. Earlier the other packages had to manouver around the missing consistency of the bit64 package. But that changes now. And later the S4 handling could be removed once the depending packages have adjusted. And then we are again a plain S3 package.

So in general, I would prefer this approach if there is no desire to become S4 anyways in the near future.

MichaelChirico · 2026-03-19T16:56:17Z

S4 handling could be removed once the depending packages have adjusted.

What should the downstream do though, exactly? I don't see a way around: as long as we export(intersect), downstream would need to do nanotime::intersect() to get their desired behavior, otherwise they need to attach the two packages in the "correct" order. What end state are you seeing?

hcirellu · 2026-03-19T17:26:40Z

In my understanding, S4 packages based on bit64 have to handle the S4 methods for their own classes, e.g. the intersect in nanotime. They have to have in mind, that integer64 is S3 and cannot generate their S4 objects, because they could be too complex and the base package does not know of the intended logic.
Since these packages depend on bit64 they import it anyways and should offer all the necessary methods regarding their own classes using setMethod. If someone attaches the bit64 package, it should not effect the behaviour of the S4 class package.

In #301 I tried to give an example, the way I work with S4. I usually strip it down to the base data type (here y@.Data), because this is the base class. After the operation I convert the result back to my object world.

setMethod("intersect", c("myInt64", "myCpl"), function(x, y) {
  # here to put the special intersect logic for nanotime and nanoival
  myCpl(intersect(x, y@.Data))
})

MichaelChirico · 2026-03-19T19:20:09Z

I'm still not seeing how that resolves this issue:

library(nanotime)
library(bit64)
intersect # bit64::intersect

hcirellu · 2026-03-20T09:15:43Z

I'm still not seeing how that resolves this issue:
library(nanotime)
library(bit64)
intersect # bit64::intersect

But this should always be the case, because bit64 overloads intersect. The overload function has to handle all cases and if one needs the former function, one has to use :: to access the function from another package.
In order to have no overloading, the S3 and S4 generics should sort that out. And since intersect ist no primitive and no generic in base, the question is what ot do.

And you want to achieve, that the intersect of nanotime is applied, if the packages are loaded in the above order?

hcirellu

I'm sorry for the late reply and that the changes in the test are all single line changes. I hope that still makes sense.

hcirellu · 2026-03-24T12:55:02Z

 #' @rdname sets
 union = function(x, y) {
+  if ((isS4(x) || isS4(y)) && isGeneric("union")) {
+    s4_intersect = selectMethod("union", list(x=class(x), y=class(y)))


Suggested change

s4_intersect = selectMethod("union", list(x=class(x), y=class(y)))

s4_union = selectMethod("union", list(x=class(x), y=class(y)))

since we are in union

hcirellu · 2026-03-24T12:55:30Z

 union = function(x, y) {
+  if ((isS4(x) || isS4(y)) && isGeneric("union")) {
+    s4_intersect = selectMethod("union", list(x=class(x), y=class(y)))
+    return(s4_intersect(x, y))


Suggested change

return(s4_intersect(x, y))

return(s4_union(x, y))

hcirellu · 2026-03-24T12:55:54Z

 #' @rdname sets
 setdiff = function(x, y) {
+  if ((isS4(x) || isS4(y)) && isGeneric("setdiff")) {
+    s4_intersect = selectMethod("setdiff", list(x=class(x), y=class(y)))


Suggested change

s4_intersect = selectMethod("setdiff", list(x=class(x), y=class(y)))

s4_setdiff = selectMethod("setdiff", list(x=class(x), y=class(y)))

hcirellu · 2026-03-24T12:56:04Z

 setdiff = function(x, y) {
+  if ((isS4(x) || isS4(y)) && isGeneric("setdiff")) {
+    s4_intersect = selectMethod("setdiff", list(x=class(x), y=class(y)))
+    return(s4_intersect(x, y))


Suggested change

return(s4_intersect(x, y))

return(s4_setdiff(x, y))

hcirellu · 2026-03-24T12:57:31Z


 #' @export
 #' @rdname sets
 is.element = function(el, set) {


Suggested change

is.element = function(el, set) {

if ((isS4(el) || isS4(set)) && isGeneric("is.element")) {

s4_is_element = selectMethod("is.element", list(el=class(el), set=class(set)))

return(s4_is_element(el, set))

}

hcirellu · 2026-03-24T13:16:11Z

+  #   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").


Suggested change

# as being is("integer64").

hcirellu · 2026-03-24T13:16:29Z

+  #   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!")


Suggested change

expect_identical(intersect(x, x), "Successfully routed to S4 method!")

hcirellu · 2026-03-24T13:16:41Z

+  #   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!")
+  expect_identical(union(x, x), "Successfully routed to S4 method!")


Suggested change

expect_identical(union(x, x), "Successfully routed to S4 method!")

hcirellu · 2026-03-24T13:16:50Z

+  #   as being is("integer64").
+  expect_identical(intersect(x, x), "Successfully routed to S4 method!")
+  expect_identical(union(x, x), "Successfully routed to S4 method!")
+  expect_identical(setdiff(x, x), "Successfully routed to S4 method!")


Suggested change

expect_identical(setdiff(x, x), "Successfully routed to S4 method!")

hcirellu · 2026-03-24T13:16:58Z

+  expect_identical(intersect(x, x), "Successfully routed to S4 method!")
+  expect_identical(union(x, x), "Successfully routed to S4 method!")
+  expect_identical(setdiff(x, x), "Successfully routed to S4 method!")
+})


Suggested change

})

hcirellu

I'm sorry for the late reply and that the changes in the test are all single line changes. I hope that still makes sense.

Attempt S4 dispatch for intersect(), maybe

9c0eedb

MichaelChirico requested a review from hcirellu March 19, 2026 06:35

MichaelChirico added 2 commits March 18, 2026 23:41

rearrange

f9ec364

corresponding test

7f4902a

MichaelChirico changed the title ~~Attempt S4 dispatch for intersect(), maybe~~ Attempt S4 dispatch for intersect() and union(), maybe Mar 19, 2026

MichaelChirico added 5 commits March 18, 2026 23:46

nanotime uses union(), too

9b5a9e7

tests for union

d556d21

(rm unneeded)

8d07b7b

also setdiff

cf211e9

setdiff tests

e11898f

MichaelChirico mentioned this pull request Mar 19, 2026

New error in {nanotime} #301

Open

hcirellu mentioned this pull request Mar 19, 2026

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

Open

hcirellu mentioned this pull request Mar 20, 2026

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

Open

hcirellu requested changes Mar 24, 2026

View reviewed changes

hcirellu reviewed Mar 24, 2026

View reviewed changes

	s4_intersect = selectMethod("union", list(x=class(x), y=class(y)))
	s4_union = selectMethod("union", list(x=class(x), y=class(y)))

	s4_intersect = selectMethod("setdiff", list(x=class(x), y=class(y)))
	s4_setdiff = selectMethod("setdiff", list(x=class(x), y=class(y)))

+is.element = function(el, set) {
+  if ((isS4(el) || isS4(set)) && isGeneric("is.element")) {
+    s4_is_element = selectMethod("is.element", list(el=class(el), set=class(set)))
+    return(s4_is_element(el, set))
+  }

Conversation

MichaelChirico commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hcirellu commented Mar 19, 2026

Uh oh!

MichaelChirico commented Mar 19, 2026

Uh oh!

hcirellu commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelChirico commented Mar 19, 2026

Uh oh!

hcirellu commented Mar 19, 2026

Uh oh!

MichaelChirico commented Mar 19, 2026

Uh oh!

hcirellu commented Mar 20, 2026

Uh oh!

hcirellu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcirellu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaelChirico commented Mar 19, 2026 •

edited

Loading

hcirellu commented Mar 19, 2026 •

edited

Loading