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 use_rng option to attribute macro extendr. #476

Closed
wants to merge 18 commits into from

Conversation

CGMossa
Copy link
Member

@CGMossa CGMossa commented Feb 5, 2023

Fixes #474.
But depends on extendr/libR-sys#123 and corresponding generated_bindings pr before merging this.
I tested this on my forks of this, and you can too!
This also depends on #469.

@CGMossa CGMossa mentioned this pull request Feb 5, 2023
4 tasks
@CGMossa CGMossa marked this pull request as draft February 6, 2023 14:17
@CGMossa
Copy link
Member Author

CGMossa commented Feb 8, 2023

First, CI is going to keep failing unless the libR-sys PRs are merged.
Second I need to finish documentation on this PR (and rebase it)

@yutannihilation
Copy link
Contributor

You can depend on the specific branch of libR-sys if we keep the pull requests on libR-sys unmerged (But, the runners without bindgen would still fail).

libR-sys = { git = "https://github.com/extendr/libR-sys" }

'libR-sys = { git = "https://github.com/extendr/libR-sys" }',

libR-sys = { git = "https://github.com/extendr/libR-sys" }

@CGMossa
Copy link
Member Author

CGMossa commented Feb 8, 2023 via email

@yutannihilation
Copy link
Contributor

It's fine if you are fine.

@CGMossa
Copy link
Member Author

CGMossa commented Feb 9, 2023

Actually, I think this is very good instruction. I think it should be in the contributor guide or something.
This PR is fairly concise, and I tested it out, so it doesn't need this (I just need to finish some documentation and for the other PRs to merge). But it was a little cumbersome to get going.

Thanks for the tip.. I didn't understand it at first. I'll write a contributor-guide PR with this in it for prosperity.

CGMossa and others added 8 commits February 10, 2023 10:49
* Started documenting the `#[extendr]` proc-macro

* fixes the need for
`use module::get_module_metadata`.

* Revert some changes

* Added an integration test.

* Fixed the need to import
the trait `GetSexp`.

* expanded the test with an adjacent module

* This should now force the extendrtests to test
this bug
* Add `CollectRArray` trait
* Document RArray::new_matrix()
* Document the robj_ndarray module
* Add conversion for owned arrays, with tests
Added `use_rng` to attribute macro
`extendr`.

Added to change-log

not done~
@CGMossa
Copy link
Member Author

CGMossa commented Mar 5, 2023

Moved to #488 due to merging chaos.

@CGMossa CGMossa closed this Mar 5, 2023
@CGMossa CGMossa deleted the extendr_rng_state branch March 5, 2023 08:17
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.

Adding random number generating
4 participants