pola-rs / r-polars

Polars R binding
https://pola-rs.github.io/r-polars/
Other
461 stars 36 forks source link

[Develop] The Makefile is used? #556

Closed eitsupi closed 8 months ago

eitsupi commented 9 months ago

Previous discussion: #173

Currently, this repository allows users to use Makefile to auto-format and run tests with a single command, but I suspect that the current state of the art is not very useful. Since the Makefile does not have the ability to record the previous execution, each time the command is run, even if the source code has not changed, the installation of the polars package and execution of all tests will take quite some time. Developers who do not like the time it takes may stop running tests.

@sorhawell @etiennebacher What do you think about this?

I was surprised to see that @etiennebacher manually edited the extendr-wrappers.R file in #554. Essentially this file should be automatically updated in the developer workflow, so I think it shows that the current workflow is not good.

As I noted in an earlier discussion, I believe Task is more appropriate than Make for this type of use, even if additional installation is required.

sorhawell commented 9 months ago

I personally use

make fmt make requirement-xyz make docs

I also use the R develop functions in https://github.com/pola-rs/r-polars/blob/main/inst/misc/develop_polars.R

load_polars(), build_polars(), check_polars() and run_all_examples_collect_errors()

load and build because they help me set environment variables and packages. check_ because it helps R CMD check to reuse previous cargo cache.

run_all_examples_collect_errors() because I don't like the devtools version. This one tries to to run all examples and collect errors. Otherwise I find myself fiddling with rerunning the examples and setting where to start from.

sorhawell commented 9 months ago

I'm open to try something else. I found our makefile ok , sometimes a bit tedious to maintain.

I agree that no auto formatted file should be manually curated beyond some prototyping. The change will likely be reverted in a future unrelated PR.

@eitsupi

How is it these days if installing from Runiverse from Linux. Does that still require clang/gcc and Make? If changing to Task, we should just make sure Task only becomes a third party dependency for developers not users. Maybe we could have some script to bootstrap a developer stack.

eitsupi commented 9 months ago

Does that still require clang/gcc and Make?

These are always necessary when installing any R source package, not just this package. In other words, there is little chance that a Linux R user will not have them.

Maybe we could have some script to bootstrap a developer stack.

In my opinion, it is not a good idea to have a script install such a tool because there are multiple ways to install it. Simply prompt them to install with documentation. (Same as Rust, Python, or R itself).

etiennebacher commented 9 months ago

even if the source code has not changed, the installation of the polars package and execution of all tests will take quite some time

This is definitely what made me not use the makefile so much. I mean I never used a makefile before so that wasn't in my usual workflow, but the few times I tried here it just took ages to compile everything. Using devtools::load_all() worked fine for me (or I thought so) but I didn't realize that the difference with rextendr::document() was that extendr-wrappers was not automatically updated. This led to the confusion in #554.

eitsupi commented 9 months ago

Thank you both. Next, I will prepare a Taskfile and compare it with the Makefile.