Skip to content

R refget bindings#172

Merged
sanghoonio merged 14 commits intodevfrom
r-workspace
Oct 3, 2025
Merged

R refget bindings#172
sanghoonio merged 14 commits intodevfrom
r-workspace

Conversation

@sanghoonio
Copy link
Copy Markdown
Member

Created R refget bindings that function similarly to the python bindings

@nleroy917
Copy link
Copy Markdown
Member

I'll take a look in detail ASAP. You'll have to update the paths/imports unfortunately since I just changed it all last night based on the entire discussion we had in the Rust channel...

But it should be pretty easy and you can just copy-paste

@sanghoonio
Copy link
Copy Markdown
Member Author

One difference from the pyo3 bindings is that you can't build R S4 classes directly in rust, and so conceptually the R bindings work a little differently for object-related things.

In refget.rs there is a function digest_fasta_raw() that returns the rust digest_fasta results as an R native list. In refget.R there is a wrapper function digest_fasta() that takes the list from rust and returns a custom S4 SequenceCollection object. Here the print methods for this object are also defined in R.

GlobalRefgetStore is a more complex object with its own rust methods, and since you can't define R S4 classes and methods in rust like pyclass and pymethods, AI suggested writing a wrapper R class and using an external pointer to a rust object in memory. So in refget.rs the bindings for GlobalRefgetStore methods are implemented as separate functions that take in the pointer to a GlobalRefgetStore object as an input. And in refget.R the wrapper R GlobalRefgetStore class has methods that call these functions. It seems to work with the example file refget_example.R I included but an issue with this approach is that if you export your R environment with this wrapper object you only save the pointer to the rust object and not the actual object, so you can't load the object back from the saved env.

Copy link
Copy Markdown
Member

@nleroy917 nleroy917 left a comment

Choose a reason for hiding this comment

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

I think these are good. Im curious about those unsafe pointers... but I think its ok for now. Especially since we aren't really distributing this even yet.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should be gitignore-ing and __pycache__ dirs

gtars-r/LICENSE Outdated
@@ -0,0 +1,2 @@
YEAR: 2024
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

2025?

read_tokens_from_gtok(&filename)
.unwrap()
.into_iter()
.map(|gtok| gtok as i32)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is i32 necessary for R? Otherwise, a u32 makes more sense here

Comment on lines +107 to +110
let store_raw_ptr = unsafe { store_ptr.external_ptr_addr::<GlobalRefgetStore>() };
if store_raw_ptr.is_null() {
return Err("Invalid store pointer".into());
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. This is the pointer stuff... is this how rextendr recommends it? I think this is probably fine honestly. I'm much more concerned with unsafe in the core crates...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Briefly, are we able to explain why this is necessary? Can we not point to a file on disk instead? Im sure there's a reason... but curious

@nleroy917
Copy link
Copy Markdown
Member

Just make sure you're in sync with the latest master!

@sanghoonio sanghoonio merged commit 1280c9c into dev Oct 3, 2025
@sanghoonio sanghoonio deleted the r-workspace branch October 9, 2025 14:13
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