BIMSBbioinfo / ciRcus

An R package for annotation of circular RNAs
10 stars 5 forks source link

follow coding style guidelines enforced by lintr #27

Closed mschilli87 closed 7 years ago

mschilli87 commented 7 years ago

The following lintr linters currently cannot be enabled without breaking the tests:

Unless we agree to skip any of those for some specifc reason, the code should be adjusted accordingly and the linters be part of the test suite.

mschilli87 commented 7 years ago

I think object_usage_linter will be hard to satisfy (see http://stackoverflow.com/q/9439256/2451238). Even though I think it makes sense (what happens if your data.frame doesn't have a column of that name?), and I'd be willing to replace aes by aes_string (see http://stackoverflow.com/a/12429344/2451238) without really solving the underlying 'problem', I have no clue how to 'fix' this for data.tables.

@retaj: Any thoughts on this?


edit: See bf41f5bd3c3a91ad0cdb2d50e3c9bdf7ab3685f0 to see what object_usage_linter complains about.

mschilli87 commented 7 years ago

I think we definitely should satisfy absolute_paths_linter, but I don't know what part of code is just some old leftover test and how to adjust the parts that cannot be deleted (yet).

@retaj: Can you have a look at 78551cb81730c98939dbd46e73eb7870b01cca6c and point me to how to work around the absolute paths?

retaj commented 7 years ago

test_load_data.R should be excluded from linting, it's just my debugging scrapbook...

object_usage_linter should be ignored (just like a bunch of R CMD check notes that get raised when it encounters ggplot2, data.table or plyr code). I know there are workarounds, but I don't think we should obfuscate our code to satisfy outdated checks and styling guidelines.

mschilli87 commented 7 years ago

My guess is that we'd rather keep these linters disabled instead of re-naming functions/variables and/or go all [[/:: (at least for now).

@retaj: Do you agree?

If so, I will remove them from the todo list of this issue and once you have resolved #30, I can finish off this one for good.

retaj commented 7 years ago

I think both dots and underscores in variable names are fine. Guess each style guide has a different preference (e.g. google style guide is strictly against underscores in R function names), but most people are comparably good at reading variable.names, variable_names and variableNames.

I would keep these two off...