ropensci / software-review

rOpenSci Software Peer Review.
292 stars 104 forks source link

plateR: tools for microtiter plate-shaped data #60

Closed seaaan closed 8 years ago

seaaan commented 8 years ago

Summary

Package: plateR
Title: Tools to Make it Easy to Work with Microtiter Plate-Shaped Data
Version: 0.2.1
Authors@R: person("Sean", "Hughes", email = "smhughes@uw.edu",
    role = c("aut", "cre"))
Description: Tools for interacting with data in the form of microtiter plates.
    plateR defines a simple, plate-shaped file format for data storage, so it's 
    easy to remember the experimental design, and provides functions to 
    seamlessly convert between that format and a tidy data frame that's optimal 
    for analysis.
Depends:
    R (>= 3.1.0)
License: GPL-3
LazyData: true
Suggests:
    testthat,
    knitr
URL: https://github.com/seaaan/plateR
BugReports: https://github.com/seaaan/plateR/issues
VignetteBuilder: knitr
Imports:
    utils,
    dplyr
RoxygenNote: 5.0.1

Confirm each of the following by checking the box. This package:

The title is not completely in lower-case. I am open to changing it to all lower-case. My rationale for the capital R was to distinguish it from the word plater.

NA

Possibly @daattali

sckott commented 8 years ago

Editor checks:


Editor comments

@seaaan Thanks for your submission!

One big thing has not bee done

ā”€  checking examples ... NONE

Please add examples for all exported functions, we'll continue review once that is done.


Reviewers: @daattali @jooolia Due date: 2016-08-23

daattali commented 8 years ago

I don't mind doing a review, but I'd be able to review the code from a technical point of view much better than practical. I have extremely limited experience with plates, I've only ever dealt with one kind (ddpcr by bio-rad) so I don't really know the ways this package could be applied. I can definitely assume that this package provides a useful utility for many people, but I myself don't know enough about plate files in the wild to judge that. For example, I looked at the sample input and it seems weird to me - is that a format that is commonly seen, or is that what users are expected to manually transform their data into?

So sure, I'll review if you think it's ok for me to do so without the domain specific knowledge :)

seaaan commented 8 years ago

Thanks to both of you for your quick responses.

@daattali: Thanks for your willingness to review, I just thought of you because of the (great!) ddPCR package. I'll leave it up to Scott/rOpenSci. To answer your question: yes, the rectangular, plate-shaped format is a super common format produced by plate readers and other instruments (though not the ddPCR droplet reader). Here's some in the wild. The user shouldn't transform their data into that format! In fact plateR has functions to deal with data in that format or in a tidy format to avoid just the copy/paste nightmare you're imagining. The user does need to do a bit of work to create the layouts corresponding to their experimental design (e.g. concentration of drug used, treatment vs. control, etc), but that's quite easy and similar to what you would do anyway when designing the experiment.

Examples

@sckott: I have added examples for all of the exported functions. Previously I had just kept them in the vignettes, since I guess that's what I personally always use when learning a new package, but adding built-in examples is definitely a good idea!

daattali commented 8 years ago

Adding examples to the functions is a good idea even if they're replicated from the vignette because that way it's easily accessible when you're inside R and are looking at the docs of a function and want to see its usage.

In regards to my question, I was wondering specifically if the format of having several "tables" inside a single csv is common, or is that what the users need to create? That's the format that seems strange to me On Jul 26, 2016 12:15 PM, "Sean Hughes" notifications@github.com wrote:

Thanks to both of you for your quick responses.

@daattali https://github.com/daattali: Thanks for your willingness to review, I just thought of you because of the (great!) ddPCR package. I'll leave it up to Scott/rOpenSci. To answer your question: yes, the rectangular, plate-shaped format is a super common format produced by plate readers and other instruments (though not the ddPCR droplet reader). Here's http://elisaanalysis.com/knowledge-base/ some in the wild. The user shouldn't transform their data into that format! In fact plateR has functions to deal with data in that format or in a tidy format to avoid just the copy/paste nightmare you're imagining. The user does need to do a bit of work to create the layouts corresponding to their experimental design (e.g. concentration of drug used, treatment vs. control, etc), but that's quite easy and similar to what you would do anyway when designing the experiment. Examples

@sckott https://github.com/sckott: I have added examples for all of the exported functions. Previously I had just kept them in the vignettes, since I guess that's what I personally always use when learning a new package, but adding built-in examples is definitely a good idea!

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ropensci/onboarding/issues/60#issuecomment-235374673, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6IFPX3GFxyhxmDMmpAQtKw5Az4LY6yks5qZlzsgaJpZM4JVTlh .

seaaan commented 8 years ago

Ah, misinterpreted your question. No, the several tables inside a single csv is not common, but it turned out to be convenient (at least for me!).

sckott commented 8 years ago

thx for adding examples @seaaan

I'll assign @daattali and look for another reviewer that has domain knowledge

jennybc commented 8 years ago

@sckott Suggest you ask around the lab groups of Wolfgang Huber or Raphael Gottardo. High-throughput phenotypic analysis and flow cytometry involve lots of data collected in micro titre plates.

Raphael G on GitHub: @raphg

Wolfgang H ... I know he's on GitHub because we co-commit to a private repo and yet I can't reach his profile! Something not quite right w/ his setup perhaps? But he's easy to find online.

Let me know if you'd like me to make a connection via email.

sckott commented 8 years ago

thanks @jennybc !

sckott commented 8 years ago

@raphg - Would you or anyone in your lab group be interested in reviewing this package?

daattali commented 8 years ago

I see there's a reviewers-assigned label, so who are the two reviewers, and what is the deadline?

sckott commented 8 years ago

sorry, edited comment above https://github.com/ropensci/onboarding/issues/60#issuecomment-235349786

thanks @jooolia and @daattali

daattali commented 8 years ago

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

Documentation

The package includes all the following forms of documentation:

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

Functionality

Final approval (post-review)

Estimated hours spent reviewing: 7


Review Comments

When I first saw the package I didn't know what it could be used for because I'm not too familiar with plate files and only have experience with a single kind, but after taking a deeper look I can 100% support this package, it can definitely be very useful for scientists dealing with plates with multiple variables. Overall this package was written, documented, and tested very well and in a very maintainable fashion.

README

Documentation

Functionality

Source code

Tests

Misc

I hope my comments make sense and are helpful. These are all of course guidelines and open to discussion, I just brain-dumped as I went through the code

seaaan commented 8 years ago

Wow, that's great, thanks so much for such a thorough review. I really appreciate it. You thought of a lot of issues I hadn't considered and made many good suggestions. It will definitely improve the package to implement your changes!

I will wait until the other review is complete to start on the changes so that the repo is an a stable state for review, but I did want to respond here to thank you for all the very clear effort you put in!

ropenscibot commented 8 years ago

@jooolia

Due date: 2016-08-23 - hey there, it's been 21 days, please get your review in soon, thanks :smiley_cat: (ropensci-bot)

daattali commented 8 years ago

The bot is a bit strict there - @jooolia still has 8 days until the deadline

sckott commented 8 years ago

@daattali that's just a reminder

daattali commented 8 years ago

ah ok, the "it's been 21 days" made it look like she's 21 days late. I like the bot though, made it much less awkward than me having to nudge my reviewer :)

jooolia commented 8 years ago

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

Documentation

The package includes all the following forms of documentation:

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

  • [X] A short summary describing the high-level functionality of the software
  • [X] Authors: A list of authors with their affiliations
  • [X] A statement of need clearly staing problems the software is designed to solve and its target audience.
  • [X] References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

Final approval (post-review)

Estimated hours spent reviewing: 5 hours

Review Comments

Overall

This package is very well documented and coded. For the package's purpose, I found that the tests were very extensive and covered many different cases. I had no installation problems under Windows and Linux. Any weird data that I could think to send through the package was met with an informative and useful error message. The package build, checks, and tests all passed on my machine. The functions, variables, and arguments are well-named and easy to understand. The vignette gives a nice overview of what the package does, and the help documentation and examples are well-written. I did not come across anything that I thought could be improved with how the package was constructed. I also thought the flexibility of plate size (# of wells) and the flexibility of the kinds of data (wells can contain numeric or character data) was very well thought-out. The only thing I would add would be documentation for plateR as a package (so that you can use help("plateR") and get the overall help menu).

Data formats

The following is solely to do with the audience and applicability and is just based on my experience. I do not want to seem overly negative, but the only downside I see for this package is that I'm not sure how many scientists will have data that look like your example data. The example data are very clear and you have made it very clear how to recreate the files. I realize that often the software used to get the data off of different machines in proprietary so it is neither practical nor possible to have an R package that would deal with all these different formats (nor is that the goal of the package!). I thought I would be able to test out this package using qPCR data that I had, but the format of my data was that either each well was a row and Cq, Dilution, SQ, etc were different columns (so already tidy) or each well is a column and each row is a cycle. Therefore to work with the second kind of data in plateR I would have to read in the excel file and then manipulate the data before using plateR (thus would likely end up with tidy data through another route). I did have some spectrophotometric data I received and sequencing plate layouts I submitted in a tidy format that I could have transformed back into a plate format using plateR, but for this to be really useful I think some sort of visualization of the values would be helpful (e.g. looking at the "microtiter" section of Bioconductor.org see packages that do this like splots or more specific qPCR viz in HTqPCR). I am not suggesting you need to do any of this, just providing comments based on my experience.

Replicates?

Another thing that is often useful and important when analysing data from microtiter plates is being able to easily group together biological and/or technical replicates. Since plateR already makes use of dplyr I could imagine that either a function or a demonstration using group_by() (or another approach) could be helpful for analysing these kinds of data.

Summary

In summary, this package really excels in its stated purpose of reading in data that are in the specific format of looking like a microtiter plate, its testing, and its documentation. Just to be clear, I do not require any changes for the package to be approved. Thanks for the chance to review!

Session info

sessionInfo()
## R version 3.3.0 (2016-05-03)
## Platform: x86_64-redhat-linux-gnu (64-bit)
## Running under: Fedora 22 (Twenty Two)
## 
## locale:
##  [1] LC_CTYPE=en_CA.UTF-8       LC_NUMERIC=C              
##  [3] LC_TIME=en_CA.UTF-8        LC_COLLATE=en_CA.UTF-8    
##  [5] LC_MONETARY=en_CA.UTF-8    LC_MESSAGES=en_CA.UTF-8   
##  [7] LC_PAPER=en_CA.UTF-8       LC_NAME=C                 
##  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
## [11] LC_MEASUREMENT=en_CA.UTF-8 LC_IDENTIFICATION=C       
## 
## attached base packages:
## [1] stats     graphics  grDevices utils     datasets  methods   base     
## 
## other attached packages:
## [1] plateR_0.2.1
## 
## loaded via a namespace (and not attached):
##  [1] R6_2.1.2        assertthat_0.1  magrittr_1.5    DBI_0.5        
##  [5] tools_3.3.0     htmltools_0.3.5 dplyr_0.5.0     tibble_1.1     
##  [9] yaml_2.1.13     Rcpp_0.12.6     stringi_1.1.1   rmarkdown_1.0  
## [13] knitr_1.14      stringr_1.0.0   digest_0.6.10   evaluate_0.9
seaaan commented 8 years ago

Thank you very much for your detailed and thoughtful review. I understand your concerns about the narrowness of the focus and will think about how to address that as I make changes in response to both reviews. Thanks again to both of you!

On Wed, Aug 17, 2016 at 8:06 AM, Julia Gustavsen notifications@github.com wrote:

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and URL, Maintainer and BugReports fields in DESCRIPTION

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly staing problems the software is designed to solve and its target audience.
  • References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 5 hours Review Comments Overall

This package is very well documented and coded. For the package's purpose, I found that the tests were very extensive and covered many different cases. I had no installation problems under Windows and Linux. Any weird data that I could think to send through the package was met with an informative and useful error message. The package build, checks, and tests all passed on my machine. The functions, variables, and arguments are well-named and easy to understand. The vignette gives a nice overview of what the package does, and the help documentation and examples are well-written. I did not come across anything that I thought could be improved with how the package was constructed. I also thought the flexibility of plate size (# of wells) and the flexibility of the kinds of data (wells can contain numeric or character data) was very well thought-out. The only thing I would add would be documentation for plateR as a package (so that you can use help("plateR") and get the overall help menu). Data formats

The following is solely to do with the audience and applicability and is just based on my experience. I do not want to seem overly negative, but the only downside I see for this package is that I'm not sure how many scientists will have data that look like your example data. The example data are very clear and you have made it very clear how to recreate the files. I realize that often the software used to get the data off of different machines in proprietary so it is neither practical nor possible to have an R package that would deal with all these different formats (nor is that the goal of the package!). I thought I would be able to test out this package using qPCR data that I had, but the format of my data was that either each well was a row and Cq, Dilution, SQ, etc were different columns (so already tidy) or each well is a column and each row is a cycle. Therefore to work with the second kind of data in plateR I would have to read in the excel file and then manipulate the data before using plateR (thus would likely end up with tidy data through another route). I did have some spectrophotometric data I received and sequencing plate layouts I submitted in a tidy format that I could have transformed back into a plate format using plateR, but for this to be really useful I think some sort of visualization of the values would be helpful (e.g. looking at the "microtiter" section of Bioconductor.org https://www.bioconductor.org/packages/release/BiocViews.html#___MicrotitrePlateAssay see packages that do this like splots https://www.bioconductor.org/packages/release/bioc/html/splots.html or more specific qPCR viz in HTqPCR https://www.bioconductor.org/packages/release/bioc/html/HTqPCR.html). I am not suggesting you need to do any of this, just providing comments based on my experience. Replicates?

Another thing that is often useful and important when analysing data from microtiter plates is being able to easily group together biological and/or technical replicates. Since plateR already makes use of dplyr I could imagine that either a function or a demonstration using group_by() (or another approach) could be helpful for analysing these kinds of data. Summary

In summary, this package really excels in its stated purpose of reading in data that are in the specific format of looking like a microtiter plate, its testing, and its documentation. Just to be clear, I do not require any changes for the package to be approved. Thanks for the chance to review! Session info

sessionInfo()

R version 3.3.0 (2016-05-03)

Platform: x86_64-redhat-linux-gnu (64-bit)

Running under: Fedora 22 (Twenty Two)

locale:

[1] LC_CTYPE=en_CA.UTF-8 LC_NUMERIC=C

[3] LC_TIME=en_CA.UTF-8 LC_COLLATE=en_CA.UTF-8

[5] LC_MONETARY=en_CA.UTF-8 LC_MESSAGES=en_CA.UTF-8

[7] LC_PAPER=en_CA.UTF-8 LC_NAME=C

[9] LC_ADDRESS=C LC_TELEPHONE=C

[11] LC_MEASUREMENT=en_CA.UTF-8 LC_IDENTIFICATION=C

attached base packages:

[1] stats graphics grDevices utils datasets methods base

other attached packages:

[1] plateR_0.2.1

loaded via a namespace (and not attached):

[1] R6_2.1.2 assertthat_0.1 magrittr_1.5 DBI_0.5

[5] tools_3.3.0 htmltools_0.3.5 dplyr_0.5.0 tibble_1.1

[9] yaml_2.1.13 Rcpp_0.12.6 stringi_1.1.1 rmarkdown_1.0

[13] knitr_1.14 stringr_1.0.0 digest_0.6.10 evaluate_0.9

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ropensci/onboarding/issues/60#issuecomment-240441588, or mute the thread https://github.com/notifications/unsubscribe-auth/AKDNdZOWD5oT_qLspnHyXdwvRyYzFtuGks5qgyNmgaJpZM4JVTlh .

sckott commented 8 years ago

Thanks for the reviews @jooolia and @daattali

@seaaan continue the conversation here

seaaan commented 8 years ago

Status update:

Just wanted to let you know where things are since it's been about 10 days. I've been steadily working through @daattali's comments and have only a few left outstanding. After that, I'll do @joolia's. I'll be on vacation for the next two weeks, which honestly means I'll probably spend more time on it, but in case I don't I wanted to let you know where things are. Thank you again for the detailed reviews and also @sckott for organizing -- it's really improving the package.

@daattali see below for responses to almost all of your comments:

Response to @daattali

README

Documentation

Functionality

d <- data.frame(Wells = c("A01", "A02", "B01", "B02"), Data = 1:4) plateR::view_plate(d, "Wells", "Data", 12) plateR::view_plate(d, "Wells", "Data", 96)

I agree that from a data visualization point of view, showing it as a 12-well plate is superior, but in terms of fidelity to what the user did in the lab, I think this would be confusing.

Source code

Misc

remaining TODOs

daattali commented 8 years ago

Great to see so much progress, and that you're really paying attention to every single comment, Everything you've done looks good, very few remaining things. I think it's ok if you take some time off during your vacation and come back to this after :)

seaaan commented 8 years ago

Sorry for the delay in response. I ended up vacationing during my whole vacation :)

@jooolia I'm planning to get to your review tomorrow. Sorry for the wait!

@daattali Thanks again for such a detailed review. Here are responses to the outstanding issues. Let me know what you think:

Thanks for the detailed examples. I split these up into several commits, shown below:

Fixed! Commit

Good idea!

Good suggestion -- I added an image of the format to the vignette. Added example to vignette

You're definitely right that the only difference between the two functions is that read_plates takes multiple files and a plate names argument while read_plate takes a single file and no plate argument. I did it this way because, as a user, I personally prefer to have more functions with fewer arguments rather than fewer functions with more optional arguments. In this case, it's true that the difference is pretty trivial. I'm inclined to leave them as separate functions but could be persuaded.

I ended up with two calls to dplyr functions: the bind_rows case you noted and then a call to select that I added for reordering columns directly below the call to bind_rows. In essence I used them for convenience and safety:

bind_rows nicely handles the case where the data frames don't have all of the same columns, which is possible in read_plates. Unfortunately do.call(rbind, list_of_df) doesn't handle that case. Of course I could write the code to do it, but I guess I trust Hadley to have figured out the possibilities better than I have.

I added the call to select for reordering of columns (specifically putting the Plate column first) which is also a pain manually and just so pleasant using select.

But I agree that these may not be sufficient reasons to import the package. Let me think about it a bit more and whether you have any other thoughts.

Could you tell me more about how you imagine using this? I can definitely do it, but I don't quite see how it would be helpful. I guess I imagine one would run "check_plateR_format()" and then if it passed, run whichever other plateR function. But all of the other functions already do the checking. I'm definitely open to doing it if there's another aspect I'm missing.

(still todo)

daattali commented 8 years ago

All my suggestions were merely suggestions and points that I wanted to bring up to your attention, so re: "read_plate vs read_plates" and "including dplyr" - I completely support you sticking with the current design if you think that makes the most sense.

check_plateR_format() - seems like it would just be a function I'd expect to exist. As a beginner user of the package, just because your package does require an input format that is non standard, I might want to see if I understand how to create the input files correctly by just getting knowing that my input files passed the validation. Again, it's your call, but as someone who is going through the readme and trying to learn the package, it was something I really wanted to use in order to prove to myself that I understand the input

seaaan commented 8 years ago

Thanks, it's very valuable feedback to know that you wished check_plateR_format() was there when you were trying to figure out the format. I added the function here and would love your feedback on whether it would be helpful.

Here are some sample calls (they'll work assuming your working directory is the package root): check_plateR_format("tests/testthat/testData/96/allWellIds.csv") check_plateR_format("tests/testthat/testData/96/incorrectRowLabels.csv") check_plateR_format("tests/testthat/testData/96/missingBottomRow.csv") check_plateR_format("tests/testthat/testData/96/missingRightColumn.csv") check_plateR_format("tests/testthat/testData/96/missingRowLabels.csv")

daattali commented 8 years ago

That's really nice, and great error reporting :+1:

seaaan commented 8 years ago

Hi @jooolia here are my responses to your review:

First of all, thank you for your insightful review and suggestions. You raised a lot of good issues and made some helpful suggestions. I really appreciate your time and effort.

Thanks for the suggestion. I added that. Most recent update

Your point is well-taken. I am certainly biased by my experience of the particular environment I work in, where I see my coworkers thrashing around in Excel with this kind of data every day. But then again, they don't use R so this package won't be useful to them either. I am interested in developing an extension of the package using Shiny to provide a web interface where my non-R coworkers (and of course whoever else wants to) can submit files to be reshaped. That is perhaps a longer term project (in terms of understanding what output shapes would be useful to them), but is something I'd like to do. I created a new issue to keep this on my mind in the future.

One thing that I struggled with in writing the vignette was trying to make it clear that plateR can be useful in situations where the instrument outputs tidy data. Specifically, I often find myself in the situation where the instrument gives me tidy data and I need to match wells with the experimental condition that they represented (control vs treatment, dilution, whatever) because I didn't enter that information into the instrument software. That's the idea of add_plate, where you match tidy data with other variables that are in plateR format. I find that much easier than trying to enter the information in tidy format because it's much more visual and close to what I actually did on the plate.

Based on your comments and the description of this file, it sounds to me like you understood that about add_plate and that the data you had was already complete with this additional information, so it wouldn't have been useful. However, if you have any suggestions as to how to change the vignette to make that aspect clearer, I'd be very glad to know.

I'm quite interested to know more about this format, would you be willing to send me a file? smhughes@uw.edu

I certainly don't think there's ever much use for transforming back into plate format (except for the relatively limited uses of view_plate). I guess my workflow for what I think you're describing would have been: create the sequencing plate layout in plate format to begin with (because it's easier than remembering which well G07 is) and then use plateR to transform to tidy format from there.

Thanks for the package suggestions! I remember looking at that section in Bioconductor in the past but I forgot about it. I agree with you that other visualizations of the values would be useful than what I have provided with view_plate. The use case that I imagine would be to get a quick overview of the data, but I would be interested to hear whether you have any further thoughts? Specifically, is there anything in particular that you can imagine being useful for your work? In the issue below, I added a quick draft of what such a function might look like and an example plot.

visualization issue

That's a good idea. It's definitely something that I do in every data analysis and providing some kind of functionality (or as you describe at least a guide) could be helpful.

replicates issue

Conclusion

I want to put up these responses for you today to get some feedback and also because it's been such a long time since you did the review that I feel guilty! But my responses do leave some loose ends.

I really like your suggestions about the visualizations and the replicates. I'd like to spend a while experimenting and figuring out what works. Rather than leave this as an open-ended process, I'm kind of inclined to go forward with the package in current form and put your new ideas into the development version for inclusion in the future, but I'm very open to what you (or anyone else) thinks on the matter. With the rest of the package, I actually experimented for a year or so before nailing down what was most useful for my process, and I like the idea of doing that with these enhancements rather than pumping them out quickly. What do you think?

jooolia commented 8 years ago

Hi @seaaan !

Thanks for the thoughtful responses. It really makes reviewing packages even more fun. I have emailed the data and commented on your viz issue. All my comments were just to share my thoughts, I did not have any problems with the package in its current state.

I think it is a very reasonable idea to release your package in its current form and then decide/work on the enhancement ideas.

seaaan commented 8 years ago

Thanks very much @jooolia and @daattali for the time and effort you put into reviewing plateR. I know the package is much better now than it was before this process.

I believe that the package is ready to go ahead with onboarding. These are the remaining outstanding issues I can think of:

Thanks again all, this was very valuable to me!

daattali commented 8 years ago

Giving my official approval, LGTM :+1:

jooolia commented 8 years ago

Me too! :+1:

sckott commented 8 years ago

thanks for the reviews @daattali and @jooolia šŸš€ āœļø šŸ“‹

@seaaan Approved!

Here are the remaining steps:

seaaan commented 8 years ago

Woohoo! I'll do this over the next few days, once it gets approved and put on CRAN. A couple questions for you @sckott:

Thanks!

On Wed, Sep 28, 2016 at 1:59 PM, Scott Chamberlain <notifications@github.com

wrote:

thanks for the reviews @daattali https://github.com/daattali and @jooolia https://github.com/jooolia šŸš€ āœļø šŸ“‹

@seaaan https://github.com/seaaan Approved!

Approved! Here are the remaining steps:

  • Close any other outstanding issues if you'd like.
  • Add the rOpenSci footer to the README
  • Transfer the repo to ropenscilabs - I've created a team in that org. account, and you should have received an email about, after you accept the invitation, go to Settings->Transfer Ownership
  • After transferring to ropenscilabs, travis builds should work automatically under the new github account. but you do need to update the link in the badge in your readme
  • Package name issue: I'd encourage you to lowercase the package name since I think the most important aspect of the name is that people will type it over and over again, and surely many will forget that the "r" should be capitalized. The case won't make a difference for google indexing it either, so that doesn't matter for find-ability of usage of your package. Decision is up to you.
  • Update all links to github repo to replace seaaan with ropenscilabs
  • I noticed one thing to deal with after running goodpractice::gp() at some point (no need to do prior to transfering though):
    • Avoid using 1:nrow(...), use seq_len() or seq_along instead

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ropensci/onboarding/issues/60#issuecomment-250297945, or mute the thread https://github.com/notifications/unsubscribe-auth/AKDNdQE3TxT_QLRKYKu6Dohcwi1DyRStks5qutUxgaJpZM4JVTlh .

sckott commented 8 years ago

If I plan to work on other outstanding issues in the future, should I do that in a fork on my own github account or in the rOpenSci repo?

Do changes in the ropenscilabs version. Treat it as you did when it was under your account.

Related, are you saying that there shouldn't be any open issues in the rOpenSci one or just that I can remove them if I want to?

No, just check to see if there's any issues you hadn't yet dealt with from these reviews

Does rOpenSci go in the DESCRIPTION?

Just in the URL and BugReports https://github.com/seaaan/plateR/blob/master/DESCRIPTION#L20-L21 fields since the url will change to .../ropenscilabs/... instead of your user name

seaaan commented 8 years ago

Okay, I transferred it over and made all of those changes! Everything seems to work. I'll submit it to CRAN tomorrow morning. Thanks.

sckott commented 8 years ago

awesome, thanks @seaaan - closing this issue now. let us know if you have any further questions.

seaaan commented 8 years ago

On CRAN! No binaries yet.

@sckott, What do I need to do for the JOSS paper?

sckott commented 8 years ago

Nice! I don't know much about JOSS. i assume you go to their website to do that now - @noamross ?

noamross commented 8 years ago

For JOSS:

noamross commented 8 years ago

Hi @seaan, just checking in, should we expect this to be submitted to JOSS?

seaaan commented 8 years ago

Thanks for the nudge @noamross -- sorry for the delay, I got sick and then lost track of it. I am planning on submitting to JOSS. I ran into a problem that I hope @sckott can help with -- to get a zenodo DOI, I need to give zenodo access to the repository, but don't control the ropenscilabs organization, so I can't. Is it possible to give it access? The instructions are here: https://help.github.com/articles/approving-third-party-applications-for-your-organization/ and here: https://guides.github.com/activities/citable-code/ If not, no worries, I'll figure something else out. Thanks!

sckott commented 8 years ago

@seaaan it looks like Zenodo is an approved 3rd party application for that org account. What happens when you try to give Zenodo access?

seaaan commented 8 years ago

Hmm, ropenscilabs/plater just doesn't show up in the list of my repositories. If I fork it, then I can get seaaan/plater to show up, so I could set it up to my fork, but ideally the DOI should point to ropenscilabs.

I found an ropenscilabs project that does have a zenodo DOI and it looks like they set it up before transferring the repo from their account to the ropenscilabs account because the DOI points to their account (which github automatically forwards to ropenscilabs).

Maybe if you transfer the plater repo back to me, I can set up the Zenodo DOI and then transfer the repo back to ropenscilabs, and github will do the forwarding. Unless you have a better idea @noamross ? Thanks both for the help and sorry for the inconvenience.

seaaan commented 8 years ago

I can also get in touch with zenodo instead and get them to help!

sckott commented 8 years ago

i changed you to admin on the repo, try again :)

seaaan commented 8 years ago

genius! It worked! Thanks :) :)

On Wed, Oct 19, 2016 at 3:03 PM, Scott Chamberlain <notifications@github.com

wrote:

i changed you to admin on the repo, try again :)

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ropensci/onboarding/issues/60#issuecomment-254954334, or mute the thread https://github.com/notifications/unsubscribe-auth/AKDNdYuF3H_3QF2_5ZWwY9RXe5aq0ahSks5q1pOqgaJpZM4JVTlh .