extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
184 stars 27 forks source link

First CRAN release #51

Closed clauswilke closed 3 years ago

clauswilke commented 3 years ago

With extendr 0.2 on crates.io, we should consider a first CRAN release of rextendr. Is there anything currently missing that we would want to add before a first release? Any API issues that should be fixed now, before the first release?

yutannihilation commented 3 years ago

Is there anything currently missing that we would want to add before a first release?

I think make_wrappers() should be exported.

andy-thomason commented 3 years ago

First of all, well done everyone for the work you have put in. I think we all feel that we have made progress.

I would feel better with a bit of a review of FromRobj. It works for the moment and could be skipped.

I feel we need some real examples of R packages, but that can wait, too.

So all in all, I would say go ahead.

yutannihilation commented 3 years ago

I feel we need some real examples of R packages, but that can wait, too.

This isn't a very real example, but for these days I've been playing with this package to mimic the stringr's functions with regex crate:

https://github.com/yutannihilation/rr4r

I think I'll write a blog post about how to create a package with extendr soon, but I don't decide whether to write it in English or in Japanese first...

clauswilke commented 3 years ago

@yutannihilation Yes, I want to export make_wrappers(), and also add a related function that automatically detects the package name, retrieves the wrapper code from the rust library, and saves it into R/extendr-wrappers.R.

Can you think of any reason why it should or should not call devtools::install() and devtools::document() as well?

devtools::install()
# Re-generate wrappers
brio::write_file(
  .Call("wrap__make_helloextendr_wrappers", use_symbols = FALSE, package_name = "helloextendr"),
  "R/extendr-wrappers.R"
)
# Re-generate documents and NAMESPACE (If you are using RStudio, just run "Document")
devtools::document()
malcolmbarrett commented 3 years ago

Are there any low-hanging package ideas for examples you'd like me to work on? Maybe a good teaching example? I'm at a point in my rust learning where I think it would be helpful to have a project, and it'd be a good excuse to learn extendr in practice

clauswilke commented 3 years ago

@malcolmbarrett If you're looking for something more ambitious, you're welcome to contribute to sinab: https://github.com/clauswilke/sinab

It needs to be ported over to the extendr framework. I expect that'll enable deletion of a lot of code. Also, sinab is missing only two features to be at feature parity with gridtext, and those two features should be simple to add(*) with extendr:

(*) Famous last words. Adding images to the layout algorithm may be non-trivial, but at least the Rust-R integration problem should be solved with extendr.

yutannihilation commented 3 years ago

Can you think of any reason why it should or should not call devtools::install() and devtools::document() as well?

In my understanding, ideally, it should not, and should be called from devtools instead. This is somehow consistent with the workflow of developing an Rcpp/cpp11 package. According to a vignette of cpp11 package:

Before building the package, you’ll need to run cpp11::cpp_register(). ...(snip)... If you are using devtools to develop your package this is done automatically by the pkgbuild package when your package has LinkingTo: cpp11 in its DESCRIPTION file.

and this is the trick I guess:

https://github.com/r-lib/pkgbuild/blob/64ce3d9e6f5b63c3aed2f255d31b9ffe20f46fcd/R/c-registration.R#L1-L14

It seems pkgbuild doesn't provide any extensible mechanism for this, but I hope we can propose some nicer one to allow us to let make_wrappers() get called.

clauswilke commented 3 years ago

Ok, will do it this way then. I think our build flow is going to be a bit different from the C++ packages because I don't think they need to be built before the registration function can be run. But anyways, that's a concern for later. I don't need to worry about it now.

andy-thomason commented 3 years ago

How is your general feeling on CRAN acceptance?

I would dearly love to start writing some real applications if only some whimsical ones like "plot your data in minecraft".

clauswilke commented 3 years ago

I have no concerns about getting rextendr onto CRAN. But just to be clear: rextendr on CRAN is neither a requirement nor a help in getting other packages with extendr onto CRAN. rextendr doesn't contain any Rust code, and it doesn't provide any features that are required to use extendr. It only helps with interactive use and writing R Markdown documents.

You could submit a package based on extendr to CRAN today. The problem you'd have to solve, though, is compilation on Windows without a Rust toolchain, and we don't have any help for that yet within the extendr framework. Some other projects have solved this, e.g. gifski. They just download a precompiled binary during the build process on CRAN: https://github.com/r-rust/gifski/blob/master/tools/winlibs.R

malcolmbarrett commented 3 years ago

Sorry, I missed your ping, @clauswilke. Sounds interesting! I'll follow-up elsewhere

clauswilke commented 3 years ago

@malcolmbarrett The sinab issue tracker is probably the best place to start: https://github.com/clauswilke/sinab/issues Just open an issue "Good places to start contributing" and we can go from there.

clauswilke commented 3 years ago

I have created a milestone for the first CRAN release and added the issues that I think must be addressed before the first release. If you can think of something else that you'd like to have included in the first release and that you can commit to implementing within the next week or so please add it to the milestone. Otherwise I'd say let's wait until the next release.

clauswilke commented 3 years ago

@yutannihilation @malcolmbarrett @Ilia-Kosenkov I'd like to go ahead and submit rextendr to CRAN. Can I do so once #116 is merged, or is there anything else critical that should happen first? Let's not worry about aspirational targets at this time, we can always make another release. I'm just interested in genuine showstoppers, if any exist.

yutannihilation commented 3 years ago

Sounds good to me. https://github.com/extendr/rextendr/issues/60 might be nice to have because the CI setup is what we actually need for package development, but I think we can release without this for now.

Ilia-Kosenkov commented 3 years ago

Please go ahead. We implemented all the features needed for the first release.

malcolmbarrett commented 3 years ago

Sounds good to me, as well. Exciting!

clauswilke commented 3 years ago

Before publishing, we may want to update the vignette on setting up a Rust build environment: https://extendr.github.io/rextendr/articles/setting_up_rust.html

Could you all review it and point out any outdated info?

Ilia-Kosenkov commented 3 years ago

@clauswilke, I can comment on the Windows section. First of all, if you do a clean install of Rust msvc toolchain on Windows, it won't work. It relies on the C++ build tools (set of compilers/linkers), which are not typically distributed with Windows. rustup-init.exe informs about this problem upon installation. I was experimenting with a script for setting up a new Windows environment. I found that these build tools can be installed unattended using choco (an unofficial Windows package manager). The problem is, I am not sure which components of build tools should be installed, the ones I selected here

``` choco install visualstudio2019buildtools -y choco install visualstudio2019-workload-vctools -y -f --package-parameters "--no-includeRecommended --add Microsoft.VisualStudio.Component.VC.CoreBuildTools --add Microsoft.VisualStudio.Component.VC.Tools.x86.x64 --add Microsoft.VisualStudio.Component.Windows10SDK.18362" ```

are enough, but they may include something which is not really needed. Rustup points to the website which distributes Visual Studio & build tools (if a standalone version is required), but that's it.

Second, we no longer need msys2 on the PATH. cargo is usually added to the PATH by default upon installation, Rtools should be configured using RTOOLS40_HOME variable, which is also set up automatically by the installer. This is all that is needed for running extendr/{rextendr}, but not for binding generation.

UPD01: And of course, msys2 installation is no longer needed. Things work with only Rtools40v2.

clauswilke commented 3 years ago

@Ilia-Kosenkov Let's leave things simple for now. We can just say "this requires a working set of C++ build tools" and let people figure out how to get those installed.

Could you make a PR fixing the msys2/path issues?

Ilia-Kosenkov commented 3 years ago

@clauswilke, sure, give me ten minutes.

clauswilke commented 3 years ago

I have submitted rextendr to CRAN. You can see the progress here: https://lockedata.github.io/cransays/articles/dashboard.html

It'll probably be a week before it gets reviewed, and then it'll get rejected for some minor reason, and then I'll have to resubmit, and then it's at least another week. So we'll have this on CRAN by the end of June if all goes well.

andy-thomason commented 3 years ago

Hooray...

Well done.

On Sat, 12 Jun 2021, 16:55 Claus Wilke, @.***> wrote:

I have submitted rextendr to CRAN. You can see the progress here: https://lockedata.github.io/cransays/articles/dashboard.html

It'll probably be a week before it gets reviewed, and then it'll get rejected for some minor reason, and then I'll have to resubmit, and then it's at least another week. So we'll have this on CRAN by the end of June if all goes well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/extendr/rextendr/issues/51#issuecomment-860072168, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAL36XH35DR7CHSPWWAOG2LTSN7QDANCNFSM4X7PIH2A .

clauswilke commented 3 years ago

Done.

https://cran.r-project.org/web/packages/rextendr/index.html