GoogleGenomics
GoogleGenomics copied to clipboard
Bioconductor
coding style
There are some competing style guides for R. Let's decide upon which one to follow or some combination of both and make sure the R package version of the client conforms to the style. (see also #17)
http://bioconductor.org/developers/how-to/coding-style/ https://google-styleguide.googlecode.com/svn/trunk/Rguide.xml
Note that the identifier naming convention of avg.clicks in the Google style guide coincides with the S3 methods naming convention and sometimes causes issues/confusion.
http://stackoverflow.com/questions/6583265/what-does-s3-methods-mean-in-r
Why not just use Google's R style checker?
https://code.google.com/p/google-rlint/
Oh, right... ;-)
Statistics is the grammar of science. Karl Pearson http://en.wikipedia.org/wiki/The_Grammar_of_Science
On Thu, Oct 30, 2014 at 11:11 AM, Nicole Deflaux [email protected] wrote:
There are some competing style guides for R. Let's decide upon which one to follow or some combination of both and make sure the R package version of the client conforms to the style. (see also #17 https://github.com/googlegenomics/api-client-r/issues/17)
http://bioconductor.org/developers/how-to/coding-style/ https://google-styleguide.googlecode.com/svn/trunk/Rguide.xml
Note that the identifier naming convention of avg.clicks in the Google style guide coincides with the S3 methods naming convention and sometimes causes issues/confusion.
http://stackoverflow.com/questions/6583265/what-does-s3-methods-mean-in-r
— Reply to this email directly or view it on GitHub https://github.com/googlegenomics/api-client-r/issues/18.
Not really. My point was that it's been "in review" for over 3 months post-useR. Not cool, especially since it was announced to hundreds of attendees as coming Real Soon Now.
Hoping to poke that with a stick (namely a "useful to public-facing GOOG projects" stick). It would, in fact, be tremendously useful and widely used if the code were available.
Statistics is the grammar of science. Karl Pearson http://en.wikipedia.org/wiki/The_Grammar_of_Science
On Thu, Oct 30, 2014 at 2:45 PM, Paul Grosu [email protected] wrote:
Thanks Tim - that's very nice and helpful! :)
— Reply to this email directly or view it on GitHub https://github.com/googlegenomics/api-client-r/issues/18#issuecomment-61175089 .
I know what you mean, but I would say the positive greatly outweighs things in this situation - so nothing really to worry about :) It's a fantastic opportunity for both codebases to grow together. Some of the best experiences I had in bringing new features to users, was when diverse teams connected with each other in the early stages of their respective projects and helped each other out. I always get excited when folks ask me, "Hey do you still have that cool example you showed me a while ago..." - I still get surprised how often they usually turn into a win-win for everybody.
~p
The problem with the rlint project is simple: there is no codebase.
https://code.google.com/p/google-rlint/source/browse/#svn%2Ftrunk%253Fstate%253Dclosed
I'd be delighted to patch any bugs but I'm not going to write something from scratch that a team of googlers announced publicly to hundreds of people. It's not going to grow if nobody puts up the code.
Either the presenters took a spot at useR which we could have given to someone else, or nobody ever followed through. Every time this issue of R style comes up, it reminds me of the conversation I had with the presenters in July. I just want them to do what they said they would. If it looks like crap, so what? I'll send patches. No code = no patches.
I hope you're right and this provides the impetus for a useful codebase to come out of hibernation. I simply can't grasp why four months isn't long enough to review a python script for release.
Just grep out the obscenities and commit it. I've certainly done that often enough.
Aha, now I got it. Looks like Andy Chen ([email protected]) is the corresponding author on the following presentation:
http://research.google.com/pubs/archive/42577.pdf
I'd be happy to send an email to get an update, unless you prefer to.
Thanks, ~p
Andy presented it, but the fellow I was talking to was handling the code review. I guess the sensible thing is for me to figure out the latter, contact both, and say "hey, we've got a public-facing use case for an R style enforcer, and it would be great not to reinvent the wheel. Can you please put the google-rlint code up, even if you have to use a pseudonym?"
Would any of the Googlers here prefer to do the honors? When I was there a million years ago, Google was pretty small and everybody knew everybody. I guess that's not the case so much anymore, but still.
It's such an obvious use case.
--t
On Oct 30, 2014, at 7:32 PM, Paul Grosu [email protected] wrote:
Aha, now I got it. Looks like Andy Chen ([email protected]) is the corresponding author on the following presentation:
http://research.google.com/pubs/archive/42577.pdf
I'd be happy to send an email to get an update, unless you prefer to.
Thanks, ~p
— Reply to this email directly or view it on GitHub.
Tim, Paul, let me reach out to the folks involved in that code. There looks to be some recent activity in it internally so it does still look active.
On Thu, Oct 30, 2014 at 8:00 PM, Tim Triche, Jr. [email protected] wrote:
Andy presented it, but the fellow I was talking to was handling the code review. I guess the sensible thing is for me to figure out the latter, contact both, and say "hey, we've got a public-facing use case for an R style enforcer, and it would be great not to reinvent the wheel. Can you please put the google-rlint code up, even if you have to use a pseudonym?"
Would any of the Googlers here prefer to do the honors? When I was there a million years ago, Google was pretty small and everybody knew everybody. I guess that's not the case so much anymore, but still.
It's such an obvious use case.
--t
On Oct 30, 2014, at 7:32 PM, Paul Grosu [email protected] wrote:
Aha, now I got it. Looks like Andy Chen ([email protected]) is the corresponding author on the following presentation:
http://research.google.com/pubs/archive/42577.pdf
I'd be happy to send an email to get an update, unless you prefer to.
Thanks, ~p
— Reply to this email directly or view it on GitHub.
— Reply to this email directly or view it on GitHub https://github.com/googlegenomics/api-client-r/issues/18#issuecomment-61209149 .
I've heard back from the engineers responsible. I don't want to make any promises, but things are moving.
On Thu Oct 30 2014 at 8:18:03 PM Paul Grosu [email protected] wrote:
Tim, that makes sense.
Dion, really appreciate it :)
— Reply to this email directly or view it on GitHub https://github.com/googlegenomics/api-client-r/issues/18#issuecomment-61210537 .
Awesome. Thanks Dion!
--t
On Oct 31, 2014, at 3:42 PM, Dion Loy [email protected] wrote:
I've heard back from the engineers responsible. I don't want to make any promises, but things are moving.
On Thu Oct 30 2014 at 8:18:03 PM Paul Grosu [email protected] wrote:
Tim, that makes sense.
Dion, really appreciate it :)
— Reply to this email directly or view it on GitHub https://github.com/googlegenomics/api-client-r/issues/18#issuecomment-61210537 .
— Reply to this email directly or view it on GitHub.
Thanks Dion - please keep us in the loop when it becomes available.
Have a great weekend! ~p
Nb. If they're really slammed for time, there's now a reasonable alternative in https://github.com/jimhester/lintr. This works for Hadley so it's another option if the google project gets stuck
Not passing judgment one way or the other, just noting recent development.
--t
On Oct 31, 2014, at 3:42 PM, Dion Loy [email protected] wrote:
I've heard back from the engineers responsible. I don't want to make any promises, but things are moving.
On Thu Oct 30 2014 at 8:18:03 PM Paul Grosu [email protected] wrote:
Tim, that makes sense.
Dion, really appreciate it :)
— Reply to this email directly or view it on GitHub https://github.com/googlegenomics/api-client-r/issues/18#issuecomment-61210537 .
— Reply to this email directly or view it on GitHub.
FWIW running lintr on the project with lintr::lint_package(linters=with_defaults(camel_case_linter = NULL)) yields the following lints.
R/client.R:88:12: style: Use <-, not =, for assignment.
messages = list()
^
R/client.R:121:38: style: Only use double-quotes.
warning(paste(messages, collapse='\n'), immediate.=TRUE)
^~~~
R/reads.R:117:1: style: lines should not be more than 80 characters.
# TODO improve performance https://github.com/googlegenomics/api-client-r/issues/17
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
R/reads.R:147:57: style: Closing curly-braces should always be on their own line, unless it's followed by an else.
cigar_enum_map[cigarPiece$operation])}),
^
R/reads.R:224:67: style: Only use double-quotes.
readsToGAlignments <- function(reads, oneBasedCoord=TRUE, slStyle='UCSC') {
^~~~~~
R/reads.R:231:26: style: Only use double-quotes.
names <- sapply(reads, '[[', 'fragmentName')
^~~~
R/reads.R:231:32: style: Only use double-quotes.
names <- sapply(reads, '[[', 'fragmentName')
^~~~~~~~~~~~~~
R/reads.R:242:3: warning: local variable ‘total_reads’ assigned but may not be used
total_reads <- length(positions)
^~~~~~~~~~~
R/reads.R:246:53: style: Only use double-quotes.
strand=strand(as.vector(ifelse(isMinusStrand, '-', '+'))),
^~~
R/reads.R:246:58: style: Only use double-quotes.
strand=strand(as.vector(ifelse(isMinusStrand, '-', '+'))),
^~~
R/variants.R:110:1: style: lines should not be more than 80 characters.
# TODO improve performance https://github.com/googlegenomics/api-client-r/issues/17
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
R/variants.R:146:69: style: Only use double-quotes.
variantsToVRanges <- function(variants, oneBasedCoord=TRUE, slStyle='UCSC') {
^~~~~~
R/variants.R:199:69: style: Only use double-quotes.
variantsToGRanges <- function(variants, oneBasedCoord=TRUE, slStyle='UCSC') {
^~~~~~
tests/runTests.R:6:36: style: Put spaces around all infix operators.
if (!is.na(apiKey) && nchar(apiKey)>0) {
~^~
tests/testthat/test-getVariants.R:3:1: style: Use two spaces to indent, never tabs.
start=50300077, end=50301500, ...)
^
tests/testthat/test-getVariants.R:16:1: style: lines should not be more than 80 characters.
"referenceBases", "alternateBases", "quality",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Thanks. We were running the google internal rlint and I missed running it on the tests directory. Looks like some of these suggestions are genuine and can be a good feedback for the rlint team. I will send an update addressing all the suggestions today.
Sid, so is the rlint code available now? If so it would be useful to have it released. Based on this link, it still seems there should be something there:
https://code.google.com/p/google-rlint/source/browse/#svn%2Ftrunk%253Fstate%253Dclosed
Thanks, Paul
Hi Paul, rlint is still not ready for public release as far as I know. Usually these projects are worked upon as side projects and so it gets very difficult to commit to hard deadlines sometimes.
Hi Sid,
No problem, that's understandable. In any case, when it does get released we can help out with it :)
Thanks, ~p