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

(inactive) PhosMap #935

Closed
8 tasks done
ecnuzdd opened this issue Nov 13, 2018 · 81 comments
Closed
8 tasks done

(inactive) PhosMap #935

ecnuzdd opened this issue Nov 13, 2018 · 81 comments

Comments

@ecnuzdd
Copy link

ecnuzdd commented Nov 13, 2018

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 understand that by submitting my package to Bioconductor,
    the package source and all review commentary are visible to the
    general public.

  • I have read the Bioconductor Package Submission
    instructions. My package is consistent with the Bioconductor
    Package Guidelines.

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

  • My package addresses statistical or bioinformatic issues related
    to the analysis and comprehension of high throughput genomic data.

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

  • The 'devel' branch for new packages and features.
  • The stable 'release' branch, made available every six
    months, for bug fixes.
  • Bioconductor version control using Git
    (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 @ecnuzdd

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: PhosMap
Type: Package
Title: A Modular Analysis Package for Phosphoproteomic Data
Version: 1.1.1
Author: Dongdong Zhan
Maintainer: Dongdong Zhan <ecnuzdd@163.com>
Description: Functions summary:
    1. Extracting the confidence probability of phosphorylation sites at peptide level from identification results searched by Mascot.
    2. Generating the quality control file of phosphorylation sites based on score of sites from Mascot.
    3. Pre-processing phosphoproteomic data.
    4. Kinase substrate enrichment analysis.
    5. Motif enrichment analysis.
    6. Data visualization
License: GPL (>= 2)
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.0.1
Date: 2018-11-07
Imports: graphics, stats, utils
Suggests: stringr, ggseqlogo, rmotifx, pheatmap, samr, limma, RankProd, ksea, e1071, ClueR, tourr

@mtmorgan
Copy link
Contributor

This package does not have a vignette, and does not re-use standard Bioconductor classes and methods for interoperability, and is therefore not suitable for Bioconductor.

@mtmorgan mtmorgan added 3b. declined not appropriate for Bioconductor and removed 1. awaiting moderation labels Nov 20, 2018
@bioc-issue-bot bioc-issue-bot changed the title PhosMap (inactive) PhosMap Nov 20, 2018
@ecnuzdd
Copy link
Author

ecnuzdd commented Dec 13, 2018

Hi, I have updated PhosMap, please help me reopen the issue to make it work.

@mtmorgan mtmorgan reopened this Dec 13, 2018
@bioc-issue-bot
Copy link
Collaborator

A reviewer has been assigned to your package. Learn what to expect
during the review process.

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.

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

A reviewer has been assigned to your package. Learn what to expect
during the review process.

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.

@mtmorgan mtmorgan added 1. awaiting moderation 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place and removed 3b. declined not appropriate for Bioconductor 1. awaiting moderation labels Dec 13, 2018
@mtmorgan mtmorgan self-assigned this Dec 13, 2018
@mtmorgan mtmorgan changed the title (inactive) PhosMap PhosMap Dec 13, 2018
@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 build report for more details.

@mtmorgan
Copy link
Contributor

Your package can depend only on CRAN or Bioconductor packages.

@bioc-issue-bot
Copy link
Collaborator

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

4b64afa update

@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 build report for more details.

@ecnuzdd
Copy link
Author

ecnuzdd commented Dec 13, 2018

Does Bioconducor not support non-CRAN or non-Bioconductor packages? The rmotifx and ksea packages are not located on CRAN and Bioconductor. In DESCRIPTION file, I wrote that: Additional_repositories: https://github.com/omarwagih/rmotifx/,https://github.com/evocellnet/ksea/

@mtmorgan
Copy link
Contributor

Packages must be on CRAN or Bioconductor, since these have at least minimal quality control and certainty of availability.

@ecnuzdd
Copy link
Author

ecnuzdd commented Dec 13, 2018

In proteome, kinase-substrate enrichment analysis (KSEA) and Motif enrichment analysis (rmotifx) are two general methods. For this case, how do I use the two packages? In DESCRIPTION file, if I modified them to "Suggests" labels, does it work? it is successful while executing 'R CMD build'.

@mtmorgan
Copy link
Contributor

It's successful from R CMD build because you have the packages installed. The build system will not install them, and they cannot be in the Depends:, Imports:, or Suggests: fields. If these packages are central to the functionality of your own package, then it means that your package cannot be accepted to Bioconductor.

@ecnuzdd
Copy link
Author

ecnuzdd commented Dec 13, 2018

Thanks for your help.
They are not core functionality but a part of my package. I will update my package by separating their functions of external packages.

@bioc-issue-bot
Copy link
Collaborator

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

6525fb1 Update DESCRIPTION

@ecnuzdd
Copy link
Author

ecnuzdd commented May 20, 2019

Hello, I have seriously check the build report. All compiling is ok except that there is only one warning when running "R CMD check".

The warnning prompts "R CMD check exceeded 10 min requirement", this is because PhosMap needs to load reference library when applying it to process Phosphoproteomics data. In our examples, we put these large files on a FTP server, once the example is tested, it first grasp reference data from FTP. The process would take a few minutes, which finally result in the warning from package builder of Biocoductor .

So, could the warning be ignored? If not, please give me some suggestion about fixing it.

@ecnuzdd
Copy link
Author

ecnuzdd commented May 21, 2019

@bioc-issue-bot Hello, I have seriously check the build report. All compiling is ok except that there is only one warning when running "R CMD check".

The warnning prompts "R CMD check exceeded 10 min requirement", this is because PhosMap needs to load reference library when applying it to process Phosphoproteomics data. In our examples, we put these large files on a FTP server, once the example is tested, it first grasp reference data from FTP. The process would take a few minutes, which finally result in the warning from package builder of Biocoductor .

So, could the warning be ignored? If not, please give me some suggestion about fixing it.

@mtmorgan
Copy link
Contributor

Have you thought of using BiocFileCache to download the data only once? Or better, to make your data available on AnnotationHub or the related ExperimentHub so that it is there even after you lose interested, and is automatically cached locally?

@ecnuzdd
Copy link
Author

ecnuzdd commented May 21, 2019

@mtmorgan Thanks for your reply. Maybe I don't describle the question clearly.

My question is that "R CMD check" would test examples written in each Rscript, the smooth execution of these partial examples is dependent on some libraries with large sizes (> 200 MB), whose loading time will result in the warnning "R CMD check exceeded 10 min requirement". These loading process does not matter where the library file exists or no matter how to cache them.

Of course, when users perform PhosMap in practice, all dependence libraries will cache locally for reusing.

I am sure that all Rscript is OK, the only warnning at the moment is "R CMD check" exceeds time while it does not come from performability of Rscript.

@mtmorgan
Copy link
Contributor

Revise the evaluated code so that it remains comprehensive but evaluates within the specified time limit. It is not satisfactory to assume that the R scripts are ok -- they may be now, but changes in your package or packages that your package depends on can break them in the future. Instead of running code on very large data, create an example data set that is enough to illustrate the analysis and to test your code.

The bottom line is that your package must build and check within specified time limits.

@bioc-issue-bot
Copy link
Collaborator

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

11ba322 Update DESCRIPTION

@bioc-issue-bot
Copy link
Collaborator

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

1690823 update for R check cmd
fd8b1cf Delete PhosMap.Rproj

@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 build report for more details.

@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 build report for more details.

@bioc-issue-bot
Copy link
Collaborator

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

a4afa7b update_examples_for_R_CMD_check

@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 build report for more details.

@bioc-issue-bot
Copy link
Collaborator

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

c51e0b0 update_examples_for_R_CMD_check_01

@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 build report for more details.

@bioc-issue-bot
Copy link
Collaborator

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

e44271b update_examples_for_R_CMD_check_01

@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 build report for more details.

@bioc-issue-bot
Copy link
Collaborator

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

152f691 Update DESCRIPTION

@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 build report for more details.

@mtmorgan
Copy link
Contributor

mtmorgan commented Jun 6, 2019

6 June, 2019

Sorry for my very slow review.

Generally, I'm concerned about the reliance on an ftp site for data in your package; past experience has been that the author does not maintain these sites in the long term, meaning that users cannot rely on package functionality. I'm also concerned about the efficiency of the code, especially the absence of vectorization and use of 'copy-and-append' as indicated below. Finally many of your examples do not evaluate because they have if(FALSE){}; this renders the examples useless and the examples should be revised so that they are fully evaluated, with only rare exceptions. Likely this means using small sample data included with the package, or data made available in ExperimentHub.

DESCRIPTION

  • good

NAMESPACE

  • Looks like this is a combination of roxygen and 'hand' generated; this is very confusing for those looking at your code. Use one style or the other.

NEWS

  • Make sure your formatting can be parsed by utils::news()

vignettes

  • All data used in your vignette must either be in your package, or in an ExperimentHub resource; it is not acceptable to download data from a private server or github

  • wrap lines carefully in code chunks so that they do not spill over page bounds on output

  • use fl <- tempfile(); dir.create(fl) rather than getwd(), ~/, or other directories in the user's file system.

  • Provide more narrative text to help the user understand the inputs and outputs of your code chunks.

  • Avoid large code chunks without intervening narrative text.

R (specific examples identified in the comments, but applicable throughout)

  • analysis_deps_anova.R:75; extract_psites_score.R:90 and many other places: avoid 'copy-and-append' pattern; instead, vectorize (best) or 'pre-allocate and fill', see http://bioconductor.org/developers/how-to/efficient-code/

  • analysis_deps_anova.R:33 use @import or @importFrom to make functions from other packages available in your package; avoid using requireNamespace().

  • use stop(), warning(), message() to signal error or informative messages to the user; use cat() only to display an object in a print() or show() method.

    if(group_nlevels < 2){
        cat('\n', 'Not found pairwise comparison.', '\n')
        stop('')
    }
    

    should be

    if(group_nlevels < 2)
        stop('Not found pairwise comparison.')
    
  • load_data_with_ftp.R:22 use BiocFileCache to manage data downloaded by the user

  • get_aligned_seq_for_mea.R:90 this code chunk looks like it could be vectorized, and could be re-written using efficient the effiicent Biostrings pacakge.

man

  • Wrap man page examples, including comment strings to 80 characters so that they display clearly in a standard console

  • Do not try to avoid evaluating example code with if (FALSE) {}, \dontrun{} or \donttest{}; if (FALSE){} is never appropriate; the latter are appropriate when a limited section of the example cannot be run by the user interactively (e.g., over-writing files) or during check (e.g., requiring user interaction).

@ecnuzdd
Copy link
Author

ecnuzdd commented Jun 10, 2019 via email

@mtmorgan
Copy link
Contributor

mtmorgan commented Jul 3, 2019

If your package is ready for further review, please be sure to increment the version to trigger a build.

@ecnuzdd
Copy link
Author

ecnuzdd commented Jul 4, 2019

Hi Martin,

Thanks for your kind reminders.

We are updating PhosMap. Its new version will be coming soon.

Best regards!

Dongdong

@mtmorgan
Copy link
Contributor

mtmorgan commented Aug 7, 2019

I'll mark this package as inactive. Comment here when you are ready to resume the review process.

@bioc-issue-bot
Copy link
Collaborator

This issue is being closed because there has been no progress
for an extended period of time. You may reopen the issue when
you have the time to actively participate in the review /
submission process. Please also keep in mind that a package
accepted to Bioconductor requires a commitment on your part to
ongoing maintenance.

Thank you for interest in Bioconductor.

@mtmorgan mtmorgan added 3c. inactive no activity from submitter for extended period of time and package not in a place to be accepted and removed 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place labels Aug 7, 2019
@lshep lshep removed the 3c. inactive no activity from submitter for extended period of time and package not in a place to be accepted label Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants