Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bioconductor style linters #43

Closed
jimhester opened this issue Feb 2, 2015 · 15 comments
Closed

Bioconductor style linters #43

jimhester opened this issue Feb 2, 2015 · 15 comments
Labels
feature a feature request or enhancement help wanted ❤️ we'd love your help! new-linter

Comments

@jimhester
Copy link
Member

Add as an optional default perhaps?

http://bioconductor.org/developers/how-to/coding-style/

@mjsteinbaugh
Copy link

@jimhester Any update on this? Would love to see this feature added. I'd be happy to help contribute.

@jimhester jimhester added feature a feature request or enhancement and removed enhancement labels May 17, 2018
@kjohnsen
Copy link

kjohnsen commented May 22, 2018

I'd also be interested in this 😃

@jimhester jimhester added the help wanted ❤️ we'd love your help! label May 23, 2018
@jimhester
Copy link
Member Author

For those interested I would gladly review a PR!

@mjsteinbaugh
Copy link

Cool thanks Jim, I'll see if I can put together a list of rules that would be helpful.

@kjohnsen
Copy link

I could use this since I'm developing a Bioconductor package now, so I'll see what I can do to help

@jimhester
Copy link
Member Author

https://bioconductor.org/developers/how-to/coding-style/ would probably be a useful place to start.

@llrs
Copy link

llrs commented Sep 27, 2018

Maybe a good way to start is using this file I could spend some time, I have also some interest on this: see r-lib/styler#331

I understand that this is different from r-lib/styler but I don't understand if there is any relationship between these packages or not. Anyway with that file it seems that it might be easier for me to use lintr

@mjsteinbaugh
Copy link

@llrs I've been using this .lintr file in my packages, which has some additional checks

@russHyde
Copy link
Collaborator

Hi @mjsteinbaugh , would you be OK if I used your .lintr for basejump to define a list of bioconductor defaults within lintr?

@mjsteinbaugh
Copy link

Sure thing!

@russHyde
Copy link
Collaborator

There's some additional best-practices mentioned here:
http://bioconductor.org/developers/package-guidelines/#rcode

  • Use vapply() instead of sapply() and use the various apply functions instead of for loops.
  • Use seq_len() or seq_along() instead of 1:...
    -Use TRUE/FALSE instead of T/F
  • Avoid class()== and class()!= instead use is()
  • Use system2() instead of system
  • Do not use set.seed in any internal R code.
  • No browser() calls should be in code
  • Avoid the use of <<-.
  • Avoid use of direct slot access with @ or slot(). Accessor methods should be created and utilized
  • Use <- instead of = for assignment
  • Function names should be camel case or utilize the underscore _ and not have a dot . which indicates S3 dispatch.
  • Use dev.new() to start a graphics drive if necessary. Avoid using x11() or X11() for it can only be called on machines that have access to an X server.

@mjsteinbaugh
Copy link

mjsteinbaugh commented Nov 12, 2019

Yes, refer to BiocCheck::BiocCheck() for details on additional Bioconductor checks. In particular, I run it on CI with these flags: https://github.com/acidgenomics/Rcheck/blob/master/checks/bioc-check

@mjsteinbaugh
Copy link

mjsteinbaugh commented Nov 12, 2019

Also, one addition that would be particularly useful in my opinion is improved handling of internal S4 method functions, which currently return as lint errors. Here's an example:
https://github.com/acidgenomics/basejump/blob/master/R/counts-methods.R#L46

counts,SummarizedExperiment will currently fail the camelCase lint check. This is similar to the S3 method lint issue reported in #223.

@MichaelChirico
Copy link
Collaborator

Encourage anyone that's interested to file PRs. Would especially be nice to include some for the 3.0.0 release. A lot of the points referenced in #43 (comment) are already doable with existing linters; others would be straightforward adaptations of existing logic (see e.g. the linters filed in #884). We can add the bioconductor tag to allow linters_with_tags("bioconductor") to facilitate easy .lintr configs.

@MichaelChirico
Copy link
Collaborator

Gonna close this issue so it's not just indefinitely open. We remain open to PRs, but none of the current maintainers are too familiar with BioConductor practices. Moving forward interested contributors might consider:

  • Filing individual PRs for supporting linters not yet available in {lintr}. If you know a particular rule you'd like to see, but are intimidated about getting started writing a new linter, please reach out & we are happy to help.
  • File feature requests for extending/parameterizing existing linters to support different styles.
  • File a new master issue listing out specific rules to support. The current issue has too much other discussion -- starting a new, focused issue with tangible end points would be more helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement help wanted ❤️ we'd love your help! new-linter
Projects
None yet
Development

No branches or pull requests

6 participants