Skip to content
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

Discordant R Package Try #2 #178

Closed
siskac opened this issue Oct 23, 2016 · 49 comments
Closed

Discordant R Package Try #2 #178

siskac opened this issue Oct 23, 2016 · 49 comments
Assignees
Labels
3a. accepted will be ingested into Bioconductor daily builder for distribution OK

Comments

@siskac
Copy link

siskac commented Oct 23, 2016

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]'

  • [x ] I understand that by submitting my package to Bioconductor,
    the package source and all review commentary are visible to the
    general public.
  • [ x] I have read the Bioconductor Package Submission
    instructions. My package is consistent with the Bioconductor
    Package Guidelines.
  • [x ] I understand that a minimum requirement for package acceptance
    is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS.
    Passing these checks does not result in automatic acceptance. The
    package will then undergo a formal review and recommendations for
    acceptance regarding other Bioconductor standards will be addressed.
  • [x ] My package addresses statistical or bioinformatic issues related
    to the analysis and comprehension of high throughput genomic data.
  • [x ] I am committed to the long-term maintenance of my package. This
    includes monitoring the support site for issues that users may
    have, subscribing to the bioc-devel mailing list to stay aware
    of developments in the Bioconductor community, responding promptly
    to requests for updates from the Core team in response to changes in
    R or underlying software.

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

  • [x ] The 'devel' branch for new packages and features.
  • [x ] The stable 'release' branch, made available every six
    months, for bug fixes.
  • [x ] Bioconductor version control using Subversion
    (optionally via GitHub).

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

@bioc-issue-bot
Copy link
Collaborator

Hi @siskac

Thanks for submitting your package. We are taking a quick
look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Package: discordant
Version: 0.99.0
Date: 2016-10-21
Title: The Discordant Method: A Novel Approach for Differential
        Correlation
Authors@R: c(person("Charlotte", "Siska", role = c("aut", "cre"), email = "charlotte.siska@ucdenver.edu"), person("Katerina", "Kechris", role = c("aut")))
Author: Charlotte Siska [cre,aut], Katerina Kechris [aut]
Maintainer: Charlotte Siska <charlotte.siska@ucdenver.edu>
Description: Discordant is a method to determine differential correlation of molecular feature pairs from -omics data using mixture models. Algorithm is explained further in Siska et al.
Encoding: latin1
biocViews: BiologicalQuestion, StatisticalMethod, mRNAMicroarray,
        Microarray, Genetics, RNASeq
Imports: stats, biwt, gtools, MASS, tools
License: GPL (>= 2)
URL: https://github.com/siskac/discordant
NeedsCompilation: yes
Packaged: 2016-10-23 17:49:47 UTC; siskac

@bioc-issue-bot
Copy link
Collaborator

Your package has been approved for building. Your package is
now submitted to our queue.

IMPORTANT: Please read the instructions for setting
up a push hook on your repository, or further changes to your
repository will NOT trigger a new build.

@bioc-issue-bot bioc-issue-bot added 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place and removed 1. awaiting moderation labels Oct 23, 2016
@bioc-issue-bot
Copy link
Collaborator

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: "ERROR".
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/discordant_buildreport_20161023171800.html

@vobencha
Copy link

vobencha commented Nov 3, 2016

Hi,
Please fix these issues raised in the Single Package Builder:

Please remove Discordant_vignette.pdf and discordant_0.99.0.tar.gz from the top level of the package.

Once that's done I'll do a full review.

Valerie

@vobencha
Copy link

Checking in. Are you planning to submit an updated version of the package?

Valerie

@siskac
Copy link
Author

siskac commented Nov 16, 2016

I’m so sorry, I keep getting hit with deadlines every time I make time to deal with this. I will try to get it done by the end of this week.

Charlotte Siska

On Nov 15, 2016, at 8:33 PM, vobencha notifications@github.com wrote:

Checking in. Are you planning to submit an updated version of the package?

Valerie


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #178 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AI73aTFpTpEpFhIt4RmHx0EZj67Vx8Ysks5q-nmdgaJpZM4KeMmx.

@siskac
Copy link
Author

siskac commented Nov 21, 2016

Hello Valerie, I’ve made the updates. I did check with Bioccheck, and the same errors are coming up. I registered to Bioconductor, fixed the man pages and pretty sure I registered my C code. Could you please look at it and tell me what to fix? Thanks.

Charlotte Siska

On Nov 15, 2016, at 8:35 PM, Charlotte Siska siskacharlotte@gmail.com wrote:

I’m so sorry, I keep getting hit with deadlines every time I make time to deal with this. I will try to get it done by the end of this week.

Charlotte Siska

On Nov 15, 2016, at 8:33 PM, vobencha <notifications@github.com mailto:notifications@github.com> wrote:

Checking in. Are you planning to submit an updated version of the package?

Valerie


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #178 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AI73aTFpTpEpFhIt4RmHx0EZj67Vx8Ysks5q-nmdgaJpZM4KeMmx.

@vobencha
Copy link

All of the errors are exactly the same as the last build report?

@siskac
Copy link
Author

siskac commented Nov 21, 2016

I see what I did wrong - I got confused where to register, and did it on the mailing list but not the support. Still getting errors about registering native routines, which I thought I did. I followed the instructions given in the link you sent. I had to take out the description of datatypes (REALSXP, INTSXP) because it was causing an error while compiling. Do you have any advice?

Thanks,
Charlotte Siska

On Nov 21, 2016, at 3:35 PM, vobencha notifications@github.com wrote:

All of the errors are exactly the same as the last build report?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #178 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AI73aSfyccFBLfiZzZ6xEXNKvekD0ezoks5rAhyUgaJpZM4KeMmx.

@vobencha
Copy link

No, without seeing the error I can't help. Please bump the version and commit to the master branch. The single package builder will produce another build and I'll see the remaining errors in that output.

For an example of a package that uses C code and registers routines look at GenomicAlignments:
svn co https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/GenomicAlignments

Valerie

@siskac
Copy link
Author

siskac commented Nov 22, 2016

I figured out how to register native routines. Thank you for the help. I just pushed the updated version onto github. Please let me know if you need anything else.

Charlotte Siska

On Nov 21, 2016, at 6:09 PM, vobencha notifications@github.com wrote:

No, without seeing the error I can't help. Please bump the version and commit to the master branch. The single package builder will produce another build and I'll see the remaining errors in that output.

For an example of a package that uses C code and registers routines look at GenomicAlignments:
svn co https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/GenomicAlignments https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/GenomicAlignments
Valerie


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #178 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AI73afmcc92I2_nEk1_ZJ9s2kb4PT946ks5rAkCxgaJpZM4KeMmx.

@vobencha
Copy link

You need to bump the version in your master github branch. This is the trigger for the single package builder. Once that's done we should see a new build report.
Valerie

@vobencha
Copy link

Can you please bump the version?

@siskac
Copy link
Author

siskac commented Nov 29, 2016 via email

@siskac
Copy link
Author

siskac commented Dec 7, 2016 via email

@vobencha
Copy link

vobencha commented Dec 7, 2016

It should have been. I'll ask Lori for a forced build.

@bioc-issue-bot
Copy link
Collaborator

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, skipped, ERROR".
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/discordant_buildreport_20161208070300.html

@siskac
Copy link
Author

siskac commented Dec 8, 2016

I searched the errors on the internet, and I found on the forum this error can happen at random. http://thread.gmane.org/gmane.science.biology.informatics.conductor.devel/9264

Should I do another build? Or is there something I can do to prevent this error in the future?

@vobencha
Copy link

vobencha commented Dec 8, 2016

You can ignore the MiKTeK errors for now. We see this warning on linux:

  • creating vignettes ... OK
    Warning: inst/doc files
    Discordant_vignette.Rnw, Discordant_vignette.pdf
    ignored as vignettes have been rebuilt.

You need to remove the inst/doc directory and the build/ directory. Bump the version and resubmit.
Valerie

@siskac
Copy link
Author

siskac commented Dec 8, 2016 via email

@bioc-issue-bot
Copy link
Collaborator

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

5dd0e6e bumped version to 0.99.2 try 2

@bioc-issue-bot
Copy link
Collaborator

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".
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/discordant_buildreport_20161209181329.html

@vobencha
Copy link

vobencha commented Dec 9, 2016

I'm able to reproduce this error on my linux system so it is real, not a problem with the builders:

  • creating vignettes ... ERROR
    Error in texi2dvi(file = file, pdf = TRUE, clean = clean, quiet = quiet, :
    Running 'texi2dvi' on 'Discordant_vignette.tex' failed.
    BibTeX errors:
    The top-level auxiliary file: Discordant_vignette.aux
    The style file: ieeetr.bst
    I couldn't open database file Discordant_bib_v3.bib
    ---line 46 of file Discordant_vignette.aux
    : \bibdata{Discordant_bib_v3
    : }
    I'm skipping whatever remains of this command
    I found no database files---while reading file Discordant_vignette.aux
    Calls: -> texi2pdf -> texi2dvi
    Execution halted

@bioc-issue-bot
Copy link
Collaborator

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

e75592a Merge branch 'release-0.99.3' into develop
37b33d1 fixed errors, version 0.99.5
51ad63b Merge branch 'release-0.99.5'
bb6cb93 updated DESC for 0.99.5
8bd078c Merge branch 'release-0.99.5'

@bioc-issue-bot
Copy link
Collaborator

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".
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/discordant_buildreport_20161211154214.html

@siskac
Copy link
Author

siskac commented Dec 11, 2016

It seems with latest version 0.99.5 there are no errors in malbec2, but still getting errors about the vignette for tokay2 and oaxaca. Please let me know how I should proceed.

@vobencha
Copy link

Hi,

We can ignore the MiKTeK errors on windows and mac for now. I've done the full review and below are some comments and suggestions.

  1. Add Depends R (>= 3.4) in DESCRIPTION; all new packages are developed against the devel branch.

  2. inputs

Biocondcutor encourages interoperability between packages. If methods in your package support (as input) the base infrastructure classes it's easier for a user to apply your method to their data.

Please have a look at the ExpressionSet, SummarizedExperiment or MultiAssayExperiment classes.

http://www.bioconductor.org/packages/3.5/bioc/html/Biobase.html

http://www.bioconductor.org/packages/3.5/bioc/html/SummarizedExperiment.html

http://www.bioconductor.org/packages/3.5/bioc/html/MultiAssayExperiment.html

One of those should be appropriate as inputs, e.g., please support one of these classes as the

  • 'mat' argument in split.MADOutlier()

  • 'x' and 'y' arguments in createVectores() and discordantRun()

  1. \details in man pages

Please add a \details section to these man pages: createVectors.Rd, discordantRun.Rd, fishersTrans.Rd and splitMADOutlier.Rd. Explain a bit about what they do and how they are unique to the package, e.g., a fisher transformation is

  1. \value in man pages

The \value section should describe the output object in general and then list specifics. Using split.MADOutlier() as an example,

\value{
\item{mat.filtered}{Input matrix where features with outliers filtered out.}
\item{index}{Index of features that have no outliers.}
}

If I run the example at the bottom of the page I get a 'mat.filtered' variable. There is no guarantee the use will name this result 'mat.filtered' so it doesn't make sense to document that name in as output in \value. The actual output is a matirx:

class(mat.filtered)

With these dimnames (what do they mean?):

dimnames(mat.filtered)
[[1]]
[1] "hsa-mir-574"

[[2]]
[1] "1" "1.1" "1.2" "1.3" "1.4" "1.5" "1.6" "1.7" "1.8" "1.9"
[11] "1.10" "1.11" "1.12" "1.13" "1.14" "2" "2.1" "2.2" "2.3" "2.4"
[21] "2.5" "2.6" "2.7" "2.8" "2.9" "2.10" "2.11" "2.12" "2.13" "2.14"
[31] "2.15" "2.16" "2.17" "2.18" "2.19" "2.20" "2.21" "2.22" "2.23" "2.24"
[41] "2.25" "2.26" "2.27" "2.28" "2.29" "2.30" "2.31" "2.32" "2.33" "2.34"
[51] "2.35" "2.36" "2.37" "2.38" "2.39" "2.40" "2.41"

I don't see an 'index' as part of the output so I'm not sure why it's documented in \value. This needs to be fixed in all function man pages not just split.MADOutlier().

  1. \arguments section on man pages

On fisherTrans.Rd the 'rhoV' input is described as a integer or numeric list. It's not a list but a vector:

class(rhoV)
[1] "numeric"
is(rhoV, "list")
[1] FALSE

Please double check the object types you've listed for the inputs.

  1. dependencies

i) Please remove this from the vignette:

\section{Required packages}
R packages WGCNA, gtools, MASS and tools are required.

When the DESCRIPTION and NAMESPACE are properly specificed you don't need to tell the use what packages discordant depends on. When biocLite("discordant") is called, all depedencies are automatically loaded.

ii) You load MASS in the fisherTrans() example for mvrnorm(). This is fine but if you expect the user to need this and other functions from MASS you could put it in 'Depends' in DESCRIPTION. I don't think the MASS package is heavy to load and it may be a good idea to make all those functions available to the user by just loading discordant.

  1. Please format your code to a max of 80 characters wide.

  2. evaluate vignette code chunks

None of your code chunks are evaluated. Instead they are wrapped in \begin{verbatim}, \end{verbatim}. Please remove the verbatim statements and evaluate the code.

We recommend the use of the BiocStyle pacakge:

http://www.bioconductor.org/packages/3.5/bioc/html/BiocStyle.html

There are vignettes on the package landing page that show how to create a pdf or html vignette.

  1. add checks for input values

I would recommend adding some checks in your code to confirm the inputs are what you expect. For example, if I give a 'groups' vector of length 2 to createVectors() no warning or error is thrown and yet the NAs are probably not what was intended. If it's expected that 'groups' is recycled that should be in the documentation.

vectors1 <- createVectors(TCGA_GBM_transcript_microarray, TCGA_GBM_miRNA_microarray, groups = groups)

sapply(vectors1, function(xx) sum(is.na(xx)))
v1 v2
0 0

vectors2 <- createVectors(TCGA_GBM_transcript_microarray, TCGA_GBM_miRNA_microarray, groups = groups[1:2])

sapply(vectors2, function(xx) sum(is.na(xx)))
v1 v2
0 200

Let me know if you have questions.

Valerie

@bioc-issue-bot
Copy link
Collaborator

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.

1 similar comment
@bioc-issue-bot
Copy link
Collaborator

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.

@vobencha
Copy link

Checking in. I'd like to finalize the package next week if at all possible.
Let me know if you have questions.
Valerie

@siskac
Copy link
Author

siskac commented Dec 29, 2016 via email

@vobencha
Copy link

@bioc-issue-bot
Copy link
Collaborator

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

7b9a67c Merge branch 'release-0.99.5' into develop
f60e485 Merge branch 'release-0.99.5' into develop
526ac77 updated R, vignette and DESC
71e03ac set DESC to 0.99.6
fd50a36 Merge branch 'release-0.99.6'
3666fcb bumped version to 0.99.6
7e5618e R updates
a5a3dc3 using ours

@bioc-issue-bot
Copy link
Collaborator

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".
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/discordant_buildreport_20170102184627.html

@bioc-issue-bot
Copy link
Collaborator

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

6e6350e cleaned up git issues
93242f4 Merge branch 'release-0.99.7'

@bioc-issue-bot
Copy link
Collaborator

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/discordant_buildreport_20170102193048.html

@siskac
Copy link
Author

siskac commented Jan 9, 2017 via email

@vobencha
Copy link

vobencha commented Jan 9, 2017

Sorry for the delay. I think all of the SPB issues have been fixed and it would be best to get a clean build before approval.

@lshep can you please kick off a new SPB for this issue?

@lshep
Copy link
Contributor

lshep commented Jan 9, 2017

Rerunning now, there should be a new build report posted shortly.

@bioc-issue-bot
Copy link
Collaborator

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/discordant_buildreport_20170109115739.html

@vobencha
Copy link

vobencha commented Jan 9, 2017

Thanks @lshep !

@siskac , the last warning to address is updating the R version,

  • Checking R Version dependency...
    • WARNING: Update R version dependency from 3.3 to 3.4.

Once that's done we are good to go.
Valerie

@bioc-issue-bot
Copy link
Collaborator

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

0a8dd9f changed R dependency

@bioc-issue-bot
Copy link
Collaborator

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings
on all platforms.

Please see the following build report for more details:

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

@siskac
Copy link
Author

siskac commented Jan 10, 2017 via email

@vobencha vobencha added 3a. accepted will be ingested into Bioconductor daily builder for distribution and removed 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place labels Jan 10, 2017
@vobencha
Copy link

vobencha commented Jan 10, 2017

It will probably take a week to post on the web site. In the next few days you'll get an email with svn credentials and instructions.

The package will only be available in devel until the next release in April 2017 after which it's available in both. For your publication you can use this link which will take you to the release page unless the package is only available in devel:
http://bioconductor.org/packages/discordant

Valerie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3a. accepted will be ingested into Bioconductor daily builder for distribution OK
Projects
None yet
Development

No branches or pull requests

5 participants