Skip to content
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.

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

Considerations for + and other special infix operators #128

Closed
DavisVaughan opened this issue Jan 10, 2022 · 4 comments · Fixed by #217
Closed

Considerations for + and other special infix operators #128

DavisVaughan opened this issue Jan 10, 2022 · 4 comments · Fixed by #217

Comments

@DavisVaughan
Copy link
Collaborator

I think one of the nice things about R7 would be to be able to write + methods which dispatch off both inputs. Like <R7Date> + <R7Duration> = <R7Date>.

I think to do this correctly in all cases we will need some base R changes.

The C code for x + y currently checks to see if there is a + method for either x or y. If there is a method for exactly one of them, or if they both have the same method, then that method is used. Otherwise if there are methods for both and they aren't the same then a fallback internal implementation is used with a warning about "incompatible methods" (See ?groupGeneric).

Practically this means that we can currently make <R7Date> + <R7Duration> work because both of them will share R7_object as a base class. We will need to write a + method for R7_object which then hands off to something that actually handles the double dispatch right (See vctrs:::+.vctrs_vctr and vctrs::vec_arith() for an example of this).

But we can't currently make <Date> + <R7Duration> work, because we'd end up with conflicting +.Date and +.R7_object methods and we'd end up getting the fallback behavior.

^ So figuring out some way to make <Date> + <R7Duration> work might be challenging, but would be a neat feature to promote about R7.

@hadley
Copy link
Member

hadley commented Jan 12, 2022

Maybe if either x or y is an R7 class, it automatically uses R7 dispatch?

@DavisVaughan
Copy link
Collaborator Author

I think that might be how it works for S4 too. This seems to try S4 dispatch if either is S4 https://github.com/wch/r-source/blob/430360aa394b02046dd7175c0fc55e347e947f80/src/main/eval.c#L3751-L3767

@hadley
Copy link
Member

hadley commented Jan 12, 2022

So implies it will already work if you create an R7 object that doesn't extend an existing base type (typeof(new_class("foo")())). (But obviously we'll also want it to work for R7 objects that do extend base types)

hadley added a commit that referenced this issue Mar 23, 2022
@hadley
Copy link
Member

hadley commented Mar 24, 2022

See also #222

hadley added a commit that referenced this issue Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants