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

Add support for tuples and better round tripping of named tuples #515

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

palday
Copy link
Collaborator

@palday palday commented Jan 4, 2024

JuliaCall currently commits some type piracy to provide support for tuples:

function sexp(x ::Tuple)
    r = sexp(Array{Any}([x...]))
    setattrib!(r, :class, sexp("JuliaTuple"))
    r
end

This PR upstreams a more efficient (and memory safe) approach that avoids the type piracy. The type piracy is particularly unfortunate because it breaks JellyMe4 when used from within JuliaCall. (JuliaCall works by starting Julia and running RCall from the Julia session.)

I also take advantage of the new tuple implementation to simplify the named tuple implementation a bit and potentially make it a bit more efficient.

Finally, I take advantage of R's class-via-attributes system to annotate the lists created from (named) tuples as coming from Julia, which makes it possible to default to constructing (named) tuples, i.e. the original types, when roundtripping them.

Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (69c22dd) 75.23% compared to head (793e12f) 75.76%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #515      +/-   ##
==========================================
+ Coverage   75.23%   75.76%   +0.53%     
==========================================
  Files          25       26       +1     
  Lines        1623     1642      +19     
==========================================
+ Hits         1221     1244      +23     
+ Misses        402      398       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

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

Makes sense AFAICT

The changes here should not be breaking for most users, but may potentially cause problems for JuliaCall , so we do a breaking version bump.
@palday palday merged commit 5d0ddeb into master Jan 8, 2024
15 checks passed
@palday palday deleted the pa/tuple branch January 8, 2024 21:19
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.

2 participants