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

plateR: tools for microtiter plate-shaped data #60

Closed
seaaan opened this Issue Jul 26, 2016 · 46 comments

Comments

Projects
None yet
7 participants
@seaaan
Contributor

seaaan commented Jul 26, 2016

Summary

  • What does this package do? (explain in 50 words or less):
    It makes it easy to convert between plate-shaped data and tidy data. Plates are used to perform experiments. Scientific instruments often give back data shaped like a plate, which is terrible for analysis. plateR lets you store plate-shaped data (easy to think about) but import tidy data (easy to analyze).
  • Paste the full DESCRIPTION file inside a code block below:
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

  • URL for the package (the development repository, not a stylized html page):
    https://github.com/seaaan/plateR
  • Who is the target audience?
    Lab scientists
  • Are there other R packages that accomplish the same thing? If so, what is different about yours?
    Not that I know of

Requirements

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

  • does not violate the Terms of Service of any service it interacts with.
  • has a CRAN and OSI accepted license.
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a vignette with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration with Travis CI and/or another service.

Publication options

Detail

  • Does R CMD check (or devtools::check()) succeed? Paste and describe any errors or warnings:
  • Does the package conform to rOpenSci packaging guidelines? Please describe any exceptions:

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.

  • If this is a resubmission following rejection, please explain the change in circumstances:

NA

  • If possible, please provide recommendations of reviewers - those with experience with similar packages and/or likely users of your package - and their GitHub user names:

Possibly @daattali

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Jul 26, 2016

Member

Editor checks:

  • Fit: The package meets or criteria fit and overlap
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  • License: The package has a CRAN or OSI accepted license
  • Repository: The repository link resolves correctly
  • Archive (JOSS only, may be post-review): The repository DOI resolves correctly
  • Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

Editor comments

@seaaan Thanks for your submission!

One big thing has not bee done

  • Examples are needed, e.g., the checking examples line from R check
─  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

Member

sckott commented Jul 26, 2016

Editor checks:

  • Fit: The package meets or criteria fit and overlap
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  • License: The package has a CRAN or OSI accepted license
  • Repository: The repository link resolves correctly
  • Archive (JOSS only, may be post-review): The repository DOI resolves correctly
  • Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

Editor comments

@seaaan Thanks for your submission!

One big thing has not bee done

  • Examples are needed, e.g., the checking examples line from R check
─  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

@sckott sckott added the holding label Jul 26, 2016

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Jul 26, 2016

Contributor

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 :)

Contributor

daattali commented Jul 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Jul 26, 2016

Contributor

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!

Contributor

seaaan commented Jul 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Jul 26, 2016

Contributor

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
#60 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6IFPX3GFxyhxmDMmpAQtKw5Az4LY6yks5qZlzsgaJpZM4JVTlh
.

Contributor

daattali commented Jul 26, 2016

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
#60 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6IFPX3GFxyhxmDMmpAQtKw5Az4LY6yks5qZlzsgaJpZM4JVTlh
.

@seaaan

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Jul 26, 2016

Contributor

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!).

Contributor

seaaan commented Jul 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Jul 26, 2016

Member

thx for adding examples @seaaan

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

Member

sckott commented Jul 26, 2016

thx for adding examples @seaaan

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

@jennybc

This comment has been minimized.

Show comment
Hide comment
@jennybc

jennybc Jul 27, 2016

Member

@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.

Member

jennybc commented Jul 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Jul 28, 2016

Member

thanks @jennybc !

Member

sckott commented Jul 28, 2016

thanks @jennybc !

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Jul 28, 2016

Member

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

Member

sckott commented Jul 28, 2016

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

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Aug 4, 2016

Contributor

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

Contributor

daattali commented Aug 4, 2016

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

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Aug 4, 2016

Member

sorry, edited comment above #60 (comment)

thanks @jooolia and @daattali

Member

sckott commented Aug 4, 2016

sorry, edited comment above #60 (comment)

thanks @jooolia and @daattali

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Aug 8, 2016

Contributor

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 recemmend approving this package.

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

  • reminder to include rOpenSci footer image after the package gets approved
  • I suppose there's no harm in README.Rmd but I'll just mention that in this repo it seems unnecessary since there's no R code in your README. You could safely delete README.Rmd and simply use a simple markdown format. But feel free to keep it if you prefer, just wanted to mention this
  • README doesn't explicitly say who the target audience is, but I feel like it's fine, it's very trivial to realize who can use it
  • There is no Maintainer field in DESCRIPTION, but I think it will get generated automatically from the "Authors@R" field maybe? @sckott maybe this could be clarified in the reviewer guidelines?

Documentation

  • I recommend adding documentation for "plateR" as the package, so that if I just want to get to the index page of the plateR documentation without a specific function in mind, I could just type "?plateR"
  • the "file" param in "read_plate" says "A character vector with the path of a..." which gives the impression that you can pass in multiple files (because of the name vector). Is this true? If you only expect one file, then I would change the wording
  • The plateR format should be explained elsewhere , not only in the read_plate function doc. The vignette does have a section about the plateR format but it only explains it in words. It would be very useful to see some examples within the vignette (I had to look at the example files under /inst/extdata in order to understand what the file should look like). Perhaps even a simple graphic
  • The documentation for plateR format for having multiple columns says "After the first plate, leave one row blank" -- by "blank" do you mean a row of commas with as many commas as there are columns? I thought blank means literally a blank line, but looking at the example csv files I see ",,,,,,,,,,,"

Functionality

  • Not clear why the # character cannot be used inside a plateR file
  • How is the user meant to generate the input file, is ot meant to be done by hand? Can there be any tool provided to help with creating this plateR file format?
  • It could be useful to have a separate function that just checks a file to see if it is in proper plateR format , since it's a format that needs to be done manually and is non standard
  • For read_plates(), I think it'd be nicer to have the plate name column as the first column rather than the last , just so that when there are many variables, it's easy to see the plate right away. You call!
  • Another "your call" suggestion: it seems to me that as a user read_plate() and read_plates() are essentially the same thing, except if there are multiple files then there is also the additional optional parameter of plate_names. If that's the only difference, it might be a bit simpler to just combine them into one function that takes either one or multiple files, instead of providing two essentially identical functions that only differ in the number of input files
  • Is there a good reason why well_ids_column has a default value of "Wells" in some functions but not in add_plate()? It would be more consistent to just always use that default value
  • add_plate(): Have you thought about making data the first param and file the second? Then you could do something like this: read.csv(file1) %>% add_plate(file2) %>% add_plate(file3) I'm not 100% sure this interaction makes sense, just wanted to throw it out there
  • The view_plate() function is really cool! Have you thought about trying to support multiple column_to_display and just showing each one on a separate line? Maybe it can be useful if you want to visualize two variables at a time?
  • It's really helpful to get an error message when reading a plate that tells me exactly in which layout there was a problem
  • Is it really necessary to provide the plate_size to view_plate()? Since the data is already provided, can't it just figure out the size?
  • There's some very good error checking in some places, but in some functions the error checking and input sanitization could be improve. For example, I shouldn't be allowed to pass well_ids_column = "" and I should get a clear error if I pass an invalid string to file or a NULL value to well_ids_column. I also noticed I get a non descriptive error if an input csv file is empty

Source code

  • In guess_plate_size: I would think it's safer to use vector[-1] instead of vector[2:length()] (for example, if the vector only has one element)
  • In get_plate_size_from_number_of_columns: I don't think the return type is meant to say that it will return TRUE
  • get_plate_size_from_number_of_columns(): maybe throwing an error makes more sense than returning a string indicating an error when an invalid number if supplied. If the only reason you returned a string is because you don't want to have an error thrown from within a nested helper function, you could use tryCatch() to see if that function resulted in an error instead of checking for the return type
  • plate_dimensions() To me it looks like random numbers , is that common accepted knowledge that plates only come in those 5 standard sizes?
  • read_plate(): it feels a bit hacky to "try adding one in case the file ends in a blank row" -- this method would also succeed is there is an extra row that isn't blank, right? Could you instead explicitly check if the last row is blank and then you know whether or not to remove it
  • I see many uses of mapply and sapply, it might be safer to try to stay away from those functions altogether
  • Really good use of helper functions
  • I think the only reason you import dplyr is for bind_row. If that's indeed the only reason, it might be worth it to just use a base equivalent and remove the dependency on such a heavy package (even though it's safe to assume most people will have it installed). I think something like do.call(rbind, list_of_df) will do the job
  • This is a very negligible comment but I can't not say it :) the validate_column_is_in_data() could take in a vector of columns and then you don't need to call it twice in a row with different columns
  • I see a lot of dataframe subsettings and very rarely a use of drop = FALSE. Have you verified that all the functions work as intended when the input file has only one well/one row/one column?

Tests

  • WOW very extensive testing! Well done, very thorough

Misc

  • I wouldn't put LICENSE in buildignore, I'd want CRAN to know about the license
  • in the YAML of paper.md I would remove the indentation that you have for all the fields
  • DESCRIPTION file: I would recommend placing the suggests, depends, and imports one after the other so that it's easy to see at a glance all the used packages, rather than having those fields in different parts of the file
  • I recommend adding dates to the NEWS file
  • the cran-comments file should specify which version of the package is submitted for each iteration of the comments (by looking at the file right now I have no idea what version is on CRAN)
  • The second sentence of the package description (in DESCRIPTION) is a big long, perhaps break it into two sentences. I noticed it's the same sentence as in the paper.md but in paper.md it sounds better because it's not the second and last sentence
  • The name of the package being "plater" vs "plateR" is a personal preference. I personally prefer all lower case to make it easier to type and so that you don't have to remember what letters are capitalized, but it's up to you (or up to the rOpenSci editors)

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

Contributor

daattali commented Aug 8, 2016

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 recemmend approving this package.

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

  • reminder to include rOpenSci footer image after the package gets approved
  • I suppose there's no harm in README.Rmd but I'll just mention that in this repo it seems unnecessary since there's no R code in your README. You could safely delete README.Rmd and simply use a simple markdown format. But feel free to keep it if you prefer, just wanted to mention this
  • README doesn't explicitly say who the target audience is, but I feel like it's fine, it's very trivial to realize who can use it
  • There is no Maintainer field in DESCRIPTION, but I think it will get generated automatically from the "Authors@R" field maybe? @sckott maybe this could be clarified in the reviewer guidelines?

Documentation

  • I recommend adding documentation for "plateR" as the package, so that if I just want to get to the index page of the plateR documentation without a specific function in mind, I could just type "?plateR"
  • the "file" param in "read_plate" says "A character vector with the path of a..." which gives the impression that you can pass in multiple files (because of the name vector). Is this true? If you only expect one file, then I would change the wording
  • The plateR format should be explained elsewhere , not only in the read_plate function doc. The vignette does have a section about the plateR format but it only explains it in words. It would be very useful to see some examples within the vignette (I had to look at the example files under /inst/extdata in order to understand what the file should look like). Perhaps even a simple graphic
  • The documentation for plateR format for having multiple columns says "After the first plate, leave one row blank" -- by "blank" do you mean a row of commas with as many commas as there are columns? I thought blank means literally a blank line, but looking at the example csv files I see ",,,,,,,,,,,"

Functionality

  • Not clear why the # character cannot be used inside a plateR file
  • How is the user meant to generate the input file, is ot meant to be done by hand? Can there be any tool provided to help with creating this plateR file format?
  • It could be useful to have a separate function that just checks a file to see if it is in proper plateR format , since it's a format that needs to be done manually and is non standard
  • For read_plates(), I think it'd be nicer to have the plate name column as the first column rather than the last , just so that when there are many variables, it's easy to see the plate right away. You call!
  • Another "your call" suggestion: it seems to me that as a user read_plate() and read_plates() are essentially the same thing, except if there are multiple files then there is also the additional optional parameter of plate_names. If that's the only difference, it might be a bit simpler to just combine them into one function that takes either one or multiple files, instead of providing two essentially identical functions that only differ in the number of input files
  • Is there a good reason why well_ids_column has a default value of "Wells" in some functions but not in add_plate()? It would be more consistent to just always use that default value
  • add_plate(): Have you thought about making data the first param and file the second? Then you could do something like this: read.csv(file1) %>% add_plate(file2) %>% add_plate(file3) I'm not 100% sure this interaction makes sense, just wanted to throw it out there
  • The view_plate() function is really cool! Have you thought about trying to support multiple column_to_display and just showing each one on a separate line? Maybe it can be useful if you want to visualize two variables at a time?
  • It's really helpful to get an error message when reading a plate that tells me exactly in which layout there was a problem
  • Is it really necessary to provide the plate_size to view_plate()? Since the data is already provided, can't it just figure out the size?
  • There's some very good error checking in some places, but in some functions the error checking and input sanitization could be improve. For example, I shouldn't be allowed to pass well_ids_column = "" and I should get a clear error if I pass an invalid string to file or a NULL value to well_ids_column. I also noticed I get a non descriptive error if an input csv file is empty

Source code

  • In guess_plate_size: I would think it's safer to use vector[-1] instead of vector[2:length()] (for example, if the vector only has one element)
  • In get_plate_size_from_number_of_columns: I don't think the return type is meant to say that it will return TRUE
  • get_plate_size_from_number_of_columns(): maybe throwing an error makes more sense than returning a string indicating an error when an invalid number if supplied. If the only reason you returned a string is because you don't want to have an error thrown from within a nested helper function, you could use tryCatch() to see if that function resulted in an error instead of checking for the return type
  • plate_dimensions() To me it looks like random numbers , is that common accepted knowledge that plates only come in those 5 standard sizes?
  • read_plate(): it feels a bit hacky to "try adding one in case the file ends in a blank row" -- this method would also succeed is there is an extra row that isn't blank, right? Could you instead explicitly check if the last row is blank and then you know whether or not to remove it
  • I see many uses of mapply and sapply, it might be safer to try to stay away from those functions altogether
  • Really good use of helper functions
  • I think the only reason you import dplyr is for bind_row. If that's indeed the only reason, it might be worth it to just use a base equivalent and remove the dependency on such a heavy package (even though it's safe to assume most people will have it installed). I think something like do.call(rbind, list_of_df) will do the job
  • This is a very negligible comment but I can't not say it :) the validate_column_is_in_data() could take in a vector of columns and then you don't need to call it twice in a row with different columns
  • I see a lot of dataframe subsettings and very rarely a use of drop = FALSE. Have you verified that all the functions work as intended when the input file has only one well/one row/one column?

Tests

  • WOW very extensive testing! Well done, very thorough

Misc

  • I wouldn't put LICENSE in buildignore, I'd want CRAN to know about the license
  • in the YAML of paper.md I would remove the indentation that you have for all the fields
  • DESCRIPTION file: I would recommend placing the suggests, depends, and imports one after the other so that it's easy to see at a glance all the used packages, rather than having those fields in different parts of the file
  • I recommend adding dates to the NEWS file
  • the cran-comments file should specify which version of the package is submitted for each iteration of the comments (by looking at the file right now I have no idea what version is on CRAN)
  • The second sentence of the package description (in DESCRIPTION) is a big long, perhaps break it into two sentences. I noticed it's the same sentence as in the paper.md but in paper.md it sounds better because it's not the second and last sentence
  • The name of the package being "plater" vs "plateR" is a personal preference. I personally prefer all lower case to make it easier to type and so that you don't have to remember what letters are capitalized, but it's up to you (or up to the rOpenSci editors)

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Aug 8, 2016

Contributor

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!

Contributor

seaaan commented Aug 8, 2016

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

This comment has been minimized.

Show comment
Hide comment
@ropenscibot

ropenscibot Aug 16, 2016

@jooolia

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

ropenscibot commented Aug 16, 2016

@jooolia

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

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Aug 16, 2016

Contributor

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

Contributor

daattali commented Aug 16, 2016

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

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Aug 16, 2016

Member

@daattali that's just a reminder

Member

sckott commented Aug 16, 2016

@daattali that's just a reminder

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Aug 16, 2016

Contributor

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 :)

Contributor

daattali commented Aug 16, 2016

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

This comment has been minimized.

Show comment
Hide comment
@jooolia

jooolia Aug 17, 2016

Contributor

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 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
Contributor

jooolia commented Aug 17, 2016

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 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
@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Aug 27, 2016

Contributor

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 :)

Contributor

daattali commented Aug 27, 2016

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Sep 25, 2016

Contributor

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:

  • More error checking For example, I shouldn't be allowed to pass well_ids_column = "" and I should get a clear error if I pass an invalid string to file or a NULL value to well_ids_column. I also noticed I get a non descriptive error if an input csv file is empty.

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

Fixed!
Commit

  • Support multiple columns in view_plate()

Good idea!

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

  • read_plate() and read_plates() are essentially the same: combine them?

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 think the only reason you import dplyr is for bind_rows. If that's indeed the only reason, it might be worth it to just use a base equivalent and remove the dependency on such a heavy package (even though it's safe to assume most people will have it installed). I think something like do.call(rbind, list_of_df) will do the job

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.

  • Add a separate function that checks a file to see if it is in proper plateR format

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.

  • reminder to include rOpenSci footer image after the package gets approved

(still todo)

Contributor

seaaan commented Sep 25, 2016

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:

  • More error checking For example, I shouldn't be allowed to pass well_ids_column = "" and I should get a clear error if I pass an invalid string to file or a NULL value to well_ids_column. I also noticed I get a non descriptive error if an input csv file is empty.

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

Fixed!
Commit

  • Support multiple columns in view_plate()

Good idea!

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

  • read_plate() and read_plates() are essentially the same: combine them?

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 think the only reason you import dplyr is for bind_rows. If that's indeed the only reason, it might be worth it to just use a base equivalent and remove the dependency on such a heavy package (even though it's safe to assume most people will have it installed). I think something like do.call(rbind, list_of_df) will do the job

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.

  • Add a separate function that checks a file to see if it is in proper plateR format

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.

  • reminder to include rOpenSci footer image after the package gets approved

(still todo)

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Sep 25, 2016

Contributor

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

Contributor

daattali commented Sep 25, 2016

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Sep 25, 2016

Contributor

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")

Contributor

seaaan commented Sep 25, 2016

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

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Sep 26, 2016

Contributor

That's really nice, and great error reporting 👍

Contributor

daattali commented Sep 26, 2016

That's really nice, and great error reporting 👍

@seaaan

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Sep 26, 2016

Contributor

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.

  • 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).

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

  • I'm not sure how many scientists will have data that look like your example data.

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.

  • Shiny issue
  • [...] the format of my data was that either each well was a row and Cq, Dilution, SQ, etc were different columns (so already tidy) [...]

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.

  • [...] or each well is a column and each row is a cycle. [...]

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

  • [...] 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 [...]

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.

  • 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).

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

  • 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.

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?

Contributor

seaaan commented Sep 26, 2016

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.

  • 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).

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

  • I'm not sure how many scientists will have data that look like your example data.

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.

  • Shiny issue
  • [...] the format of my data was that either each well was a row and Cq, Dilution, SQ, etc were different columns (so already tidy) [...]

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.

  • [...] or each well is a column and each row is a cycle. [...]

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

  • [...] 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 [...]

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.

  • 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).

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

  • 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.

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

This comment has been minimized.

Show comment
Hide comment
@jooolia

jooolia Sep 27, 2016

Contributor

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.

Contributor

jooolia commented Sep 27, 2016

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Sep 27, 2016

Contributor

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:

  • Submit to CRAN (it's ready to go, I'm just waiting in case there are any last minute changes that I forgot about)
  • Add the rOpenSci image to the README (just waiting for official approval)
  • Resolve the plater/plateR naming issue (I would prefer to leave it as plateR, but if lower-case is a strong preference of rOpenSci, I'm okay with changing it. What do you think @sckott?)
  • ???

Thanks again all, this was very valuable to me!

Contributor

seaaan commented Sep 27, 2016

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:

  • Submit to CRAN (it's ready to go, I'm just waiting in case there are any last minute changes that I forgot about)
  • Add the rOpenSci image to the README (just waiting for official approval)
  • Resolve the plater/plateR naming issue (I would prefer to leave it as plateR, but if lower-case is a strong preference of rOpenSci, I'm okay with changing it. What do you think @sckott?)
  • ???

Thanks again all, this was very valuable to me!

@daattali

This comment has been minimized.

Show comment
Hide comment
@daattali

daattali Sep 27, 2016

Contributor

Giving my official approval, LGTM 👍

Contributor

daattali commented Sep 27, 2016

Giving my official approval, LGTM 👍

@jooolia

This comment has been minimized.

Show comment
Hide comment
@jooolia

jooolia Sep 28, 2016

Contributor

Me too! 👍

Contributor

jooolia commented Sep 28, 2016

Me too! 👍

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Sep 28, 2016

Member

thanks for the reviews @daattali and @jooolia 🚀 ✏️ 📋

@seaaan 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
Member

sckott commented Sep 28, 2016

thanks for the reviews @daattali and @jooolia 🚀 ✏️ 📋

@seaaan 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
@seaaan

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Sep 29, 2016

Contributor

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

  • 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? 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?
  • Does rOpenSci go in the DESCRIPTION?

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
#60 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKDNdQE3TxT_QLRKYKu6Dohcwi1DyRStks5qutUxgaJpZM4JVTlh
.

Contributor

seaaan commented Sep 29, 2016

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

  • 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? 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?
  • Does rOpenSci go in the DESCRIPTION?

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
#60 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKDNdQE3TxT_QLRKYKu6Dohcwi1DyRStks5qutUxgaJpZM4JVTlh
.

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Sep 29, 2016

Member

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

Member

sckott commented Sep 29, 2016

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Oct 4, 2016

Contributor

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

Contributor

seaaan commented Oct 4, 2016

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

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 4, 2016

Member

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

Member

sckott commented Oct 4, 2016

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

@sckott sckott closed this Oct 4, 2016

@seaaan

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Oct 6, 2016

Contributor

On CRAN! No binaries yet.

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

Contributor

seaaan commented Oct 6, 2016

On CRAN! No binaries yet.

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

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 6, 2016

Member

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

Member

sckott commented Oct 6, 2016

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

@noamross

This comment has been minimized.

Show comment
Hide comment
@noamross

noamross Oct 6, 2016

Collaborator

For JOSS:

  • Archive your package in a long-term repository with a DOI. We recommend Zenodo. Here are instructions: https://guides.github.com/activities/citable-code/
  • Check off the DOI box and add the DOI at the top of this review
  • Submit at http://joss.theoj.org/papers/new
  • We will alert the JOSS editor that this is an rOpenSci package, so they can make an expedited decision based on this thread rather than assign new reviewers.
Collaborator

noamross commented Oct 6, 2016

For JOSS:

  • Archive your package in a long-term repository with a DOI. We recommend Zenodo. Here are instructions: https://guides.github.com/activities/citable-code/
  • Check off the DOI box and add the DOI at the top of this review
  • Submit at http://joss.theoj.org/papers/new
  • We will alert the JOSS editor that this is an rOpenSci package, so they can make an expedited decision based on this thread rather than assign new reviewers.
@noamross

This comment has been minimized.

Show comment
Hide comment
@noamross

noamross Oct 19, 2016

Collaborator

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

Collaborator

noamross commented Oct 19, 2016

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

@seaaan

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Oct 19, 2016

Contributor

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!

Contributor

seaaan commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 19, 2016

Member

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

Member

sckott commented Oct 19, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Oct 19, 2016

Contributor

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.

Contributor

seaaan commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Oct 19, 2016

Contributor

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

Contributor

seaaan commented Oct 19, 2016

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

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 19, 2016

Member

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

Member

sckott commented Oct 19, 2016

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

@seaaan

This comment has been minimized.

Show comment
Hide comment
@seaaan

seaaan Oct 19, 2016

Contributor

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
#60 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKDNdYuF3H_3QF2_5ZWwY9RXe5aq0ahSks5q1pOqgaJpZM4JVTlh
.

Contributor

seaaan commented Oct 19, 2016

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
#60 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKDNdYuF3H_3QF2_5ZWwY9RXe5aq0ahSks5q1pOqgaJpZM4JVTlh
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment