Test "verbose" option for R.

[rcurtin]

As suggested by @eddelbuettel here, I wrote a simple test to ensure that when you pass verbose = TRUE to an R binding, you get some kind of output at all.

I'm not R-fluent, but this seems to be correct? I could have misunderstood something 😄

[eddelbuettel]

That looks promising. I may be a build behind (i.e. at the CRAN version) so I get no effect from verbose yet. Maybe try also to assign to a var to split out the stdout side effect:

## CRAN version so fails
> expect_output(res <- test_r_binding(4.0, 12, "hello", build_model=TRUE, verbose=TRUE))
Error: `res <- test_r_binding(4, 12, "hello", build_model = TRUE, verbose = TRUE)` produced no output
> 

Edit: I may actually declared legally blind but you already had exactly that. Never mind the fellow chiming in from IL then.

[coatless]

@rcurtin perfect unit test! The expect_output() and expect_silent() handle the verbose option output standard quite well.

Another option would be to include a snapshot test regarding the log information, e.g. expect_snapshot(); however, the logging output is tested by the main mlpack library.

[eddelbuettel]

Looks good to me!

[coatless]

@eddelbuettel to test you'll need to grab a copy of the mlpack tarball from the runner as we didn't push this feature onto CRAN yet.

mlpack.mlpack / mlpack R tarball (pull_request)

[eddelbuettel]

@coatless Indeed and I kinda know and said so.

[rcurtin]

Yep, I think 4.4.0 is coming soon so I'll just rebuild for CRAN then. (Like, this week? Next week? With the documentation work progressing, we are quickly getting to somewhere where automatic website rebuilds are easy, past which point the manual labor associated with each release gets a good bit lower, which in turn means it's easier to do releases for each minor feature instead of letting them stack up. Of course, we should be careful how much we believe me here, since I've been vaguely saying things along these lines for nearly a decade or more...)

[eddelbuettel]

Like, this week?

R 4.4.0 is due on Tuesday so this week may be a good week to sit out. Or not. But "they" will be busy. So yes, rolling to next week may work best. On the other hand per https://cloud.r-project.org/web/checks/check_results_mlpack.html the package is in perfect standing, and has just a handful of reverse depends so you may get the auto-treatment by the bot ... So could also try on Friday, say.

[coatless]

@rcurtin I think the main driver/power user @cgiachalis is okay with using a nightly mlpack build.

While this is a good issue to solve, it isn't show-stopping in using the package within a compiled code context (which triggered the out-of-version release).

If need be, I can also handle another out-of-point release mid-week.

[rcurtin]

Up to you, if you want to release mlpack on CRAN feel free, it's pretty easy (but you already know this), you can just take the tarball straight out of the most recent Github Actions build on the master branch, set the version appropriately, and away it goes.

[eddelbuettel]

Yeah but you're the maintainer so it needs the email handshake.

We should rely more on r-universe. I can build for tags, release, or (by default) all pushes to the default branch. We rely a ton on it at work and with our partners at CZI.

[cgiachalis]

@coatless I am ok with nightly build.

[rcurtin]

If it's easy to set up on this repository and not too intrusive, let's do it---we can set it up to run when new tags are pushed, that seems like it would be the best option? (And like it would avoid the email handshake?)

[coatless]

@rcurtin nope; still need the e-mail handshake to go to CRAN.

What I think Dirk is advocating for here is being able to make the nightlies more accessible via R-universe (a separate entity from CRAN).

[eddelbuettel]

I just added it to my r-universe as a firsts (and with the proper "subdir" argument). @coatless: Do we know if it builds via remotes::install_github() from a subdir? That is what r-universe needs ...

PS So see here in a bit. Has hourly pulses, plus as we know a build of mlpack takes a moment or two.

[coatless]

@eddelbuettel unfortunately, the rendered mlpack bindings never enter the repo. So, the subdir portion I'm not sure would work.

That said, I know that r-universe supports subdir inside of the packages.json due to the work with Garrick on {quarto-countdown} needing the countdown repository to be reoriented. c.f.

	{
		"package": "countdown",
		"url": "https://github.com/gadenbuie/countdown",
		"branch": "main",
		"subdir": "r"
	},

gadenbuie/gadenbuie.r-universe.dev@cc85248

[eddelbuettel]

@coatless My question was not about subdir; we already use that as well as the other qualifier branch extensively in other r-universes. My question was explicitly to you about this repo: do you know if anyone has ever built via remotes?

subdir is no deal-breaker per se but you have to make sure that the hooks at the build directory you designate either bring the other files in (that is what the various arrow adbc* packages seem to do) or you have for example a softlink as we do in one. I guess we will find out soon enough :) But yeppers, pointing into monorepos such as this one can have its challenges.

[eddelbuettel]

So far I only got a red 'x' with nothing to show for in terms of logs: https://github.com/r-universe/eddelbuettel/actions/runs/8791049719/job/24124413368

My added paragraph is fairly innocent:

  {
    "package": "mlpack",
    "maintainer": "Ryan Curtin <ryan@ratml.org>",
    "url": "https://github.com/mlpack/mlpack",
    "subdir": "src/mlpack/bindings/R/mlpack",
    "available": true
  },

But I may have to lift it again.

@rcurtin This is likely something we want to look into and get going. Builds for both macOS hardware type, windows and even for Ubuntu LTS are neat. I may some use of that eg for r-polars which otherwise is hard to get / tedious to build and we effectively use this to binaries to our users for work.

[rcurtin]

I can provide support, but I don't really have the bandwidth to be the main driver on this one. I don't understand the R universe system (and it's ok, I don't need to), but I wonder if it makes more sense to make the upload a part of the R tarball job?

https://github.com/mlpack/mlpack/blob/master/.github/workflows/main.yml

The mlpack repository itself is likely to give more headache than anything, since the entire R package is automatically built and doesn't actually exist in the repository, only in the build artifacts.

I definitely agree this would be nice to have though, especially if it allows distributing binaries. A word of caution though: binary build systems are often nightmare points of friction---if you want to see the pain, take a look at the https://github.com/mlpack/mlpack-wheels repository, which I use to build Python wheels for PyPI, and is a constant mess of "why did this particular build fail" and "let me kick this again". I'd really prefer to not be the owner of another system like that, but this time for R.

[coatless]

@eddelbuettel whoops missed that portion. I don't think this repository has ever been built with remotes::install_github(). The bindings require a top-level cmake routine to generate the R package and, then, the usual R package shenanigans to install.

Maybe we should ping Jeroen Ooms for advice on the best strategies for a nontraditional repo before continuing?

[eddelbuettel]

I hear you, maybe more than you know. $work is more of a Python than an R shop and I am fully aware what resources we devote to e.g. maintain Conda builds. Pypi is relatively easy compared to it ... but r-universe is a walk in the park! It is 'simply' that this type of use has not been a concern so there may be a facet we have not used. That said, it is otherwise just like CRAN: it takes the sources, run R CMD build to create a .tar.gz and installs it. 'All we have to do' (if we wanted to take advantage) is to ensure that works off the repo as checked out in the main branch (or any other branch or tag or ... git reference we point at). It's a good (free) option to have.

[eddelbuettel]

@coatless Re

The bindings require a top-level cmake routine to generate the R package

that is all a non-issue. Jeroen has cmake present, and we have the other files present as the repo is 'complete'. Maybe all you and I need to riff on then in to maybe decide in configure if we are in fact in a r-universe build (env.var MY_UNIVERSE) and trigger whatever needs triggering.

[eddelbuettel]

Ok, subsequent fails had better logs:

Adding new package 'mlpack' from: https://github.com/mlpack/mlpack
Cloning into '/tmp/RtmpOcAAgO/eddelbuettel-universe/mlpack'...
ERROR file 'mlpack/src/mlpack/bindings/R/mlpack/DESCRIPTION' does not exist 
No packages to delete. Everything is up-to-date

ERROR updating mlpack from https://github.com/mlpack/mlpack (file 'mlpack/src/mlpack/bindings/R/mlpack/DESCRIPTION' does not exist)

So we get shot down because we do not have a DESCRIPTION file as a marker of an R repo -- we only have DESCRIPTION.in. Sad but true.

[eddelbuettel]

Good news re r-universe (and maybe we want to carry that over to a new issue?) as I heard from @jeroen (when I asked about tricks to pre-empt a DESCRIPTION.in and fill in some way or other):

Yes you can either add a .prepare shell script or similarly bootstrap.R script that is run after checkout before CMD build https://github.com/r-universe-org/build-source/blob/master/entrypoint.sh#L44-L58

That seems like a way forward. Haven't had time to peruse his entrypoint.sh in detail yet though.

[coatless]

I think the build is failing because of:

`... <- NULL` produced no output

causing:

Error ('testthat.R:4:1'): (code run outside of `test_that()`)

[rcurtin]

Oops! I just realized that the test_r_binding() test function doesn't actually print any output at all. I pushed ed77e91, which should hopefully fix the build.

[coatless]

Tests pass now for R! Though, I'm surprised that just the output line was crashing testthat.