Recommend styler over formatR suggestion

[lcolladotor]

Hi,

Based on my recent work for r-lib/actions#84 and at https://github.com/leekgroup/derfinderPlot/blob/master/dev/02_update.R, I want to make a small PR to BiocCheck and http://www.bioconductor.org/developers/how-to/coding-style/. This is directly related to r-lib/styler#636.

While formatR has been really helpful, in my experience when I use it, I need to double check very carefully all the code as it might change the meaning of some of it. My understanding is that styler is the package that is best maintained now for styling code (you still need to check your code! but styler seems to do a better job) and I quickly stumbled upon it yesterday on a Google search. Unlike formatR, it's compatible with tidyverse code (magrittr pipes and multi-line piped code), it also updates the example code in roxygen2-formatted examples. For my derfinderPlot package (a small one I was testing things with), it didn't break any code unlike formatR (see r-lib/styler#636 for the quotes issue).

styler has a styler::tidyverse_style() function and at r-lib/styler#636 I'm suggesting creating a styler::bioc_style() or styler::bioc_like_style() function that builds upon their styler::tidyverse_function(). Even without such a function, you can currently use styler like I did so at https://github.com/leekgroup/derfinderPlot/blob/master/dev/02_update.R#L2-L12 when updating my derfinderPlot package. Hopefully they like the idea, at which point, I would make a PR in styler. Probably styler::bioc_like_style() is the better name since I wouldn't want the function to enforce things like camelCase, somefunc(a=1, b=2) over somefunc(a = 1, b = 2), but well, those aren't really checked by BiocCheck anyway.

Both styler and formatR have issues with line lengths as noted in r-lib/styler#247, but from what I read there, it's a very complicated problem.

For BiocCheck, if you like the idea, I'm suggesting making a PR that either recommends styler first and formatR second, or only recommends styler (given that styler seems to me more actively maintained). I would also like to make the same suggestion for http://www.bioconductor.org/developers/how-to/coding-style/ though I don't know if there's a GitHub repo for the Bioc website.

Best,
Leo

[lcolladotor]

Ahh, I see that https://github.com/Bioconductor/bioconductor.org exists!

[lshep]

Would gladly accept pull requests!

[lshep]

Bioconductor/Contributions#1451 (comment) Proof -- this package very much liked it

[lcolladotor]

Awesome! Thanks for letting me know Lori =)

I've been working on the getting GitHub Actions up and running for bioc packages at r-lib/actions#84 and this is my current status of check-bioc.yml from leekgroup/derfinderPlot@d586ae4. I'm in the process of applying this to several packages. Next I need to address Bioconductor/Contributions#1389 (comment) :P

Once I'm done with that, I'll get back to this ^_^

[mtmorgan]

my own feeling is that one gets 90% of the way there by implementing any commonly accepted style, without having to re-invent the wheel or quibble about details. Of course my own 'priority' is 4 versus two space indentation, and I'd be happy with a recommendation (on bioconductor.rog / BiocCheck; does it require a bioc_like_style()?

style_pkg(".", transformers=tidyverse_style(indent = 4))

[lcolladotor]

Hi,

As I described at r-lib/styler#636 (comment), I made a new R package called biocthis that implements the bioc_style() function. You can then use it like I have at https://github.com/lcolladotor/biocthis/blob/fe66af4a12334559fc7d7968e53de0e676cb8689/dev/04_update.R#L9-L22. That is:

## Automatically re-style the code in your package to a Bioconductor-friendly
## format
## Note that you can pair this function with the RStudio "Reformat code"
## button on the magic wand drop down menu. The keyboard shortcut is
## macOS: shift + command + A
## Windows: shift + control + A
styler::style_pkg(transformers = biocthis::bioc_style())
styler::style_dir(here::here("dev"), transformers = biocthis::bioc_style())
styler::style_dir(
    here::here("vignettes"),
    transformers = biocthis::bioc_style(),
    filetype = "Rmd"
)
styler::style_file(here::here("README.Rmd"), transformers = biocthis::bioc_style())

biocthis::bioc_style() is fairly simple and very similar to styler::tidyverse_style(indent = 4).

For the PR, would it be ok to mention both? If so, I'll send the PR soon here and then the one for Bioconductor/bioconductor.org#54 (comment).

Best,
Leo

[mtmorgan]

I like the idea of biocthis, but think that it is outside the scope of this hackathon.

Also I think it is not suitable as a pull request for BiocCheck, because it adds functionality (correcting problems) beyond the purpose of the package. In the process it introduces significant additional dependencies for relatively specialized function.

FWIW this pull request might find a better home in something like biocthis, which could have dependencies on packages (like projroot, ...), etc, that are not relevant to the main purpose of the package (BiocStyle).

How about developing biocthis as a separate collaborative project, maybe seeded with discussion on the slack developer forum? I could imagine the dependency analysis and build / check badge generation functions finding a home there, too.

[lcolladotor]

Hi,

Clarifications

I think that we might have had some misunderstandings.

The PR I'm proposing for BiocCheck doesn't involve actual code, just the message that has the recommendations on the tools to use for addressing formatting issues. I wanted to ask whether it was ok to edit this message to mention https://lcolladotor.github.io/biocthis/articles/biocthis.html#bioc-style- or if it should only mention styler. That is:

styler::style_pkg(transformers = biocthis::bioc_style()) vs styler::style_pkg(transformers = styler::tidyverse_style(indent = 4)).

So this and Bioconductor/bioconductor.org#54 are really about documentation changes.

It might be best if I just send the PR to get this process started. I initiated this issue before the upcoming hackathon was announced, but I could wait until then if you prefer that option.

biocthis

It's become my "fun" (lots of debugging) side project since April that I've been working on the weekends. This past one I wrote the documentation for it (https://lcolladotor.github.io/biocthis/) and I'm planning on writing a blog post about it. Prompted by what you just said, I'll share the blog post on the Bioc Slack too. As it stands, it could also submit biocthis for formal peer review at Bioc (it now passes BiocCheck too).

BiocStyle/pull/76

I haven't looked in detail at the code in Bioconductor/BiocStyle#76, but yes, this could live in biocthis. It would complement (two functions to keep it simpler) the one I implemented at https://lcolladotor.github.io/biocthis/reference/use_bioc_vignette.html.

Best,
Leo

[mtmorgan]

Absolutely then updating BiocCheck to advocate for a specific tool for reformatting is in scope.

I think there is value in continuing to think of this (this issue) as part of the hackathon. At the least I think it would be valuable to provide other community members to hear / discuss this if desired...

[lcolladotor]

Ok! I'll wait for the remote hackathon next week then =) I like the idea of getting community feedback.

[lcolladotor]

In response to the 2020-05-18 BiocCheck-a-thon intro session: Count me in for resolving this issue (can you assign me to it? I can't self-assign). If anyone else is interested, please let me know. Thanks!