search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

gCrisprTools #67

Closed OscarBrock closed 8 years ago

OscarBrock commented 8 years ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

Confirm the following by editing each check box to '[x]'

I am familiar with the essential aspects of Bioconductor software management, including:

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 8 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/gCrisprTools_buildreport_20160815072028.html

lshep commented 8 years ago

Thank you for your recent updates. Please see the following comments:

GENERAL:

  1. Was the build directory created automatically or can this be deleted?
  2. Please delete the legacy DESCRIPTION~

FROM REPORT:

  1. Please update the R dependency to 3.3
  2. It seems you have added invalid biocViews: Crispr, PooledScreesn. Were you attempting to add biocViews or were you using them as keywords? Invalid biocViews are not allowed.

MAN:

  1. (ct.alignmentChart, ct. generateResults, ct.guide.CDF, ct.prepareAnnotation) It seems in several man pages you reference ExpressionPlot and functions like ep.alignment.class.counts or ep.load.annot. Where are these from? There is no package reference to know where these came from.
  2. ct.gRNARankByRepliate : where is calcNormFactors from? Can you add more details in the description or a reference link for the argument lib.size on what is voom-appropriate library size adjustments?
  3. ct.normalizeGuide : consider adding a \seealso{ } section with the mentioned normalization functions.

VIGNETTE: Crispr_example_workflow.R

  1. Please remove "::" shouldn't need these because of the library calls.
  2. ct.GCbias(es, ann, sk) cannot be run. It looks like this function was never exported in the NAMESPACE

gCrisprTools_Vignette.R

  1. Section 5.1. Can you add more description on what RRAaggregation is? In the man pages you also reference RRAa but don't go into any detail on the algorithm or where it came from.

Thank you for your time and effort. I think with these modifications the package will be close to acceptance.

OscarBrock commented 8 years ago

Hi Lori,

Thanks for your continued help with this.

Yes, I was hoping to add some new biocViews. I asked around the department about the right way to submit suggestions, and the R gurus recommended that I do it this way; if there is a better way for me to suggest new biocViews I'm happy to use whatever approach you recommend.

All of your comments are clear and reasonable. I'll clarify the source of those functions as you suggest, and improve the documentation of RRAa in the vignette specifically; I agree that it is hard to track down a clear explanation in the various ancillary documents and man pages. The Expression Plot mentions are included in error- they refer to an internal R package that is used at our institution, but isn't available to most Bioc users. I'll go through again and purge the remainder of these references.

Best,

-R

On Tue, Aug 16, 2016 at 7:51 AM, lshep notifications@github.com wrote:

Thank you for your recent updates. Please see the following comments:

GENERAL:

  1. Was the build directory created automatically or can this be deleted?
  2. Please delete the legacy DESCRIPTION~

FROM REPORT:

  1. Please update the R dependency to 3.3
  2. It seems you have added invalid biocViews: Crispr, PooledScreesn. Were you attempting to add biocViews or were you using them as keywords? Invalid biocViews are not allowed.

MAN:

  1. (ct.alignmentChart, ct. generateResults, ct.guide.CDF, ct.prepareAnnotation) It seems in several man pages you reference ExpressionPlot and functions like ep.alignment.class.counts or ep.load.annot. Where are these from? There is no package reference to know where these came from.
  2. ct.gRNARankByRepliate : where is calcNormFactors from? Can you add more details in the description or a reference link for the argument lib.size on what is voom-appropriate library size adjustments?
  3. ct.normalizeGuide : consider adding a \seealso{ } section with the mentioned normalization functions.

VIGNETTE: Crispr_example_workflow.R

  1. Please remove "::" shouldn't need these because of the library calls.
  2. ct.GCbias(es, ann, sk) cannot be run. It looks like this function was never exported in the NAMESPACE

gCrisprTools_Vignette.R

  1. Section 5.1. Can you add more description on what RRAaggregation is? In the man pages you also reference RRAa but don't go into any detail on the algorithm or where it came from.

Thank you for your time and effort. I think with these modifications the package will be close to acceptance.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/67#issuecomment-240126412, or mute the thread https://github.com/notifications/unsubscribe-auth/AObUqcBKRGOMJwVWwoU_Pn-zg10doZt5ks5qgc55gaJpZM4JQJAZ .

lshep commented 8 years ago

Where were you thinking the two terms should be located?: see biocViews

OscarBrock commented 8 years ago

Most likely in the Technology tree, perhaps with Pooled Screens on the Sequencing branch.

-R

On Tue, Aug 16, 2016 at 9:58 AM, lshep notifications@github.com wrote:

Where were you thinking the two terms should be located?: see biocViews http://bioconductor.org/packages/devel/BiocViews.html#___Software

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/67#issuecomment-240166630, or mute the thread https://github.com/notifications/unsubscribe-auth/AObUqbvYD0fdOv81pdUbl49T_A_-bAhDks5qgew-gaJpZM4JQJAZ .

lshep commented 8 years ago

We have added the new biocViews: CRISPR and PooledScreens. They will become available after our next build (~48 hrs). Please be aware they are case sensitive to adjust if necessary.

bioc-issue-bot commented 8 years ago

Received a valid push; starting a build. Commits are:

3e76ad0 Internal version 1.53 New updates in accordance w... e5cf6e2 Merge pull request #7 from OscarBrock/Bioc-Revisio...

bioc-issue-bot commented 8 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/gCrisprTools_buildreport_20160824153956.html

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

Received a valid push; starting a build. Commits are:

541c3e0 Update DESCRIPTION

bioc-issue-bot commented 8 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/gCrisprTools_buildreport_20160824162526.html

OscarBrock commented 8 years ago

Hi, still catching that error on the windows machine (I'm emailing because I'm not sure if you hear about builds that fail). Hopefully everything else is shipshape now...

-R

On Wed, Aug 24, 2016 at 1:23 PM, bioc-issue-bot notifications@github.com wrote:

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/gCrisprTools_ buildreport_20160824162526.html

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/67#issuecomment-242195955, or mute the thread https://github.com/notifications/unsubscribe-auth/AObUqQUZmbEsV4JzD65AxgfbZ2MnRHXJks5qjKhZgaJpZM4JQJAZ .

lshep commented 8 years ago

Thank you for making these changes. The package looks okay now and I will recommend be accepted. You should get svn instructions via email in the next couple of days.

OscarBrock commented 8 years ago

Excellent! Thanks so much for your careful work and patience.

-R

On Tue, Aug 30, 2016 at 10:32 AM, lshep notifications@github.com wrote:

Thank you for making these changes. The package looks okay now and I will recommend be accepted. You should get svn instructions via email in the next couple of days.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/67#issuecomment-243517120, or mute the thread https://github.com/notifications/unsubscribe-auth/AObUqWcscArN8Gev_kQDDzOPCMxXyQgLks5qlGkxgaJpZM4JQJAZ .

OscarBrock commented 8 years ago

Hi Lori,

I wanted to follow up on next steps for submitting the package to the svn...

Thanks,

-R

On Tue, Aug 30, 2016 at 10:32 AM, lshep notifications@github.com wrote:

Thank you for making these changes. The package looks okay now and I will recommend be accepted. You should get svn instructions via email in the next couple of days.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/67#issuecomment-243517120, or mute the thread https://github.com/notifications/unsubscribe-auth/AObUqWcscArN8Gev_kQDDzOPCMxXyQgLks5qlGkxgaJpZM4JQJAZ .

lshep commented 8 years ago

Sorry for the delay you should be receiving instructions soon.