Skip to content

Implement of the RClass idea #192

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

Merged
merged 4 commits into from
Jun 14, 2017
Merged

Implement of the RClass idea #192

merged 4 commits into from
Jun 14, 2017

Conversation

randy3k
Copy link
Member

@randy3k randy3k commented Jun 13, 2017

close #138

An example of usage can be found in datatime.jl.

@richardreeve, I guess you may be interested in it.

@randy3k randy3k changed the title Implement of the Class idea Implement of the RClass idea Jun 13, 2017
@randy3k randy3k requested a review from simonbyrne June 13, 2017 19:43
@richardreeve
Copy link

Great, I have a brutalist version of my code (EcoJulia/Phylo.jl#3) that uses this and it seems to work using @rget, though I'm having to create RObjects again in a definitely-wrong sort of way. Are there plans for exporting RClass and rcall_p, which I currently seem to need, or should I just use call using on them specifically?

@randy3k
Copy link
Member Author

randy3k commented Jun 13, 2017

RClass and rcall_p are not meant to be used by normal users. I guess we should keep them unexported.

@richardreeve
Copy link

Would it be crazy to create a submodule like RCall.Extend for things like protect, unprotect, rcall_p, RClass, isObject, isS4 to make it easy to extend the conversion functionality without having loads of using statements?

@randy3k
Copy link
Member Author

randy3k commented Jun 13, 2017

There are too many functions falling into this category. I am not sure if it becomes necessary if most of the internal functions are put in say, RCall.API

@richardreeve
Copy link

Yes, I assumed there would be lots of them tbh, which was why I thought something like RCall.API would be useful. Obviously there's also a second much smaller group of functions that need importing too - rcopy, rcopytype, sexp...

@richardreeve
Copy link

So, I'm completely happy with this, and I don't want to open a can of worms, but when multiple people start providing converters for the same R type, is there going to be a way forward? As things stand (if I understand Julia multiple dispatch correctly, which I probably don't!) are they going to overwrite one another, with the last man standing? That may be fine of course, but I thought I ought to raise it...

@randy3k
Copy link
Member Author

randy3k commented Jun 13, 2017

I believe only the last overload counts (untested). I guess there is no easy fix for this. Since there should be one and only one output, it should not be very bad to use the last known type.

@richardreeve
Copy link

Yes, that's what I thought. I guess it's okay - it would be a strange situation using two different Julia types in a single piece of code that match the same R type (and wanting to do conversions!).

@randy3k randy3k merged commit f23103e into JuliaInterop:master Jun 14, 2017
@randy3k randy3k deleted the RClass branch June 14, 2017 00:41
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 this pull request may close these issues.

support for passing custom types?
3 participants