lorenzwalthert / precommit

pre-commit hooks for R projects
https://lorenzwalthert.github.io/precommit/
GNU General Public License v3.0
248 stars 48 forks source link

style-files fails with unused arguments #586

Open deeplook opened 1 month ago

deeplook commented 1 month ago

This is likely a feature request...

I'm trying to run these pre-commit hooks on existing R code, but I get errors like this which (to me as a non-R-expert) appear to be due to the fact that the R code is not packaged and does not use renv (and hence there is no, renv.lock file):

style-files..............................................................Failed
- hook id: style-files
- exit code: 1

- The project is out-of-sync -- use `renv::status()` for details.
- The project is out-of-sync -- use `renv::status()` for details.
NULL
[1] "cache root set to " "styler-perm"       
Error in style(...) : unused arguments (type = "R", `NA` = NULL)
Calls: tryCatch ... <Anonymous> -> transform_files -> make_transformer -> force
Execution halted
- The project is out-of-sync -- use `renv::status()` for details.
- The project is out-of-sync -- use `renv::status()` for details.
NULL
[1] "cache root set to " "styler-perm"       
Error in style(...) : unused arguments (type = "R", `NA` = NULL)
Calls: tryCatch ... <Anonymous> -> transform_files -> make_transformer -> force
Execution halted

A nice solution would be to just run these hooks on plain R code without having to turn it into a full-fledged package.

lorenzwalthert commented 1 month ago

Please follow the issue template outlined in this repo.

lorenzwalthert commented 1 month ago

actually there was a loop hole and you said you don't know exactly what this is so I give you the benefit of the doubt 😀.

lorenzwalthert commented 1 month ago

It's a misconception that you need to use {renv} in your repo to use pre-commit, pre-commit itself uses a renv to manage the hooks, but you should not see it, since the renv is not put in your project directory. So you can use a renv or not to manage your project's dependencies, that does not matter.

I think the source of your problem is that you pass invalid arguments to the hook. Please paste your .pre-commit-config.yaml here.

lorenzwalthert commented 1 month ago

A nice solution would be to just run these hooks on plain R code without having to turn it into a full-fledged package.

It's not a requirement that your code repo is an R package. I am not sure where you got his this impression from. If from the docs, I'd appreciate a pointer so we can move the confusion out of the way.

lorenzwalthert commented 1 month ago
  • The project is out-of-sync -- use renv::status() for details.

I can't explain these, since the renv gets installed once in pre-commit's cache and then should be frozen, so I don't see how this can get out of date except if you change installed versions yourself in this renv, or change the renv.lock or similar.

deeplook commented 1 month ago

I know that pre-commit runs the hooks inside its own dedicated environment. But for some reason it seems to cause an error on my five R files in an otherwise unstructured directory. So it was not immediately clear to me as a non-R person what it means, making me speculate I had to turn the code into a real R package, which I would not expect to be a mandatory requirement, after having used pre-commit on many Python projects before.

So here is my .pre-commit-config.yaml file, and sorry for not having posted it upfront:

repos:
  - repo: https://github.com/lorenzwalthert/precommit
    rev: v0.4.3
    hooks:
      - id: style-files
        args: [--type, R]
deeplook commented 1 month ago

In fact I've run the hook now on a dummy (first) R project of mine (also not using renv) if I followed the tutorial right. You can see the same error in the respective GitHub action: https://github.com/deeplook/fibonacciPackage/actions/runs/10194682116/job/28201779778

lorenzwalthert commented 1 month ago

The supported parameters you can supply thorough args: are specified in the docs, args: does not take --type or R as arguments. Don't know where you picked that up. If you want to stick to default tidyerse style, you can just omit the line.

repos:
  - repo: https://github.com/lorenzwalthert/precommit
    rev: v0.4.3
    hooks:
      - id: style-files
lorenzwalthert commented 1 month ago

in the .pre-commit-hooks.yaml (not to be confused with .pre-commit-config.yaml), style-files specified how the hook should be invoked:

-   id: style-files
    name: style-files
    description: style files with {styler}
    entry: Rscript inst/hooks/exported/style-files.R
    language: r
    files: '(\.[rR]profile|\.[rR]|\.[rR]md|\.[rR]nw|\.[qQ]md)$'
    exclude: 'renv/activate\.R'
    minimum_pre_commit_version: "2.13.0"

If you want, you can override files in your .pre-commit-config.yaml. Maybe that's what you tried to achieve?

deeplook commented 1 month ago

Thanks! TBH, I forgot where I got my snippet from. If I remove the arguments, the styling is indeed applied, which is great! There is just the same comments about renv that might (and do) confuse noob R users like me. But thanks a lot!

% pre-commit run --all-files   
style-files..............................................................Failed
- hook id: style-files
- files were modified by this hook

- The project is out-of-sync -- use `renv::status()` for details.
- The project is out-of-sync -- use `renv::status()` for details.
NULL
[1] "cache root set to " "styler-perm"       
Styling  1  files:
 R/fibonacci.R ℹ 
────────────────────────────────────────
Status  Count   Legend 
✔       0       File unchanged.
ℹ       1       File changed.
✖       0       Styling threw an error.
────────────────────────────────────────
Please review the changes carefully!

% pre-commit run --all-files
style-files..............................................................Passed
lorenzwalthert commented 1 month ago

Great. You can force a re-installation with with pre-commit clean and then install the hooks again. Does tie problem persist on that case?

deeplook commented 1 month ago

Unfortunately, that doesn't help.