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

convert KEGGgraph.Rnw to KEGGgraph.Rmd #16

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sonali8434
Copy link

@jwokaty kindly Review this PR before we ask @Accio for a final review.

@Accio We noticed that this pdf contains sections referring to pages, but it is not possible to refer to pages in new .Rmd file since we don't have a page no in an HTML document. So we have referred to sections instead of pages in the new KEGGgraph.Rmd file. If you have any suggestions about this, please don't hesitate to let us know.

We generally ask one of our team members to review the conversions before asking the maintainers to review them.

@Accio Generally, we give credits to the converter for converting Sweave documents. We ask maintainers if they want us to add ourselves as contributors in the description file and the. Rmd file. I wanted to ask if you would like me to add as a contributor for this conversion. It's totally fine to say no.

After your final review when you merge the PR, remember to bump the version in the DESCRIPTION file so that the Bioconductor build system will recognize the new changes and push the changes to Bioconductor's git repository.

well as a collection of tools to analyze, dissect and visualize these graphs.

The package requires KGML (KEGG XML) files, which can be downloaded from KEGG
REST web service ((https://rest.kegg.jp/)) without license permission for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have one set of parenthesis.

Comment on lines 91 to 92
* Manual download from KEGG REST API, documented at at (https://www.kegg.jp/kegg/rest/keggapi.html).
* Automatic retrieval from KEGG REST API, available at (https://rest.kegg.jp),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the parentheses from URLs in lines 91 and 92.

```

Pathways and their identifiers can be browsed in the KEGG website
(https://genome.jp/kegg), or found with any search engine.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove parenthesis

The subgraph is visualized in figure [1](#fig:01), where nodes with in-degree or
out-degree in red and others in grey [^1]. And in the example we also
demonstrate how to convert KEGG ID into other other identifiers via the Entrez
GeneID. More details on the conversion of IDs can be found on section [Annotation](#convertId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's improve the wording a little with in the [Annotation section](#convertId). and add a full stop at the end.

To convert GeneID to other identifiers, we recommend genome wide annotation
packages, for human it is `r Biocpkg("org.Hs.eg.db")` and the packages for other
organisms can be fount at
(http://www.bioconductor.org/packages/release/data/annotation/). To demonstrate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove parenthesis

packages, for human it is `r Biocpkg("org.Hs.eg.db")` and the packages for other
organisms can be fount at
(http://www.bioconductor.org/packages/release/data/annotation/). To demonstrate
its use, we draw the sub-network in the figure [2](fig:02) again, whereas nodes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link doesn't work; maybe it's missing the #?

Comment on lines +322 to +324
*Note*: the current version of `RBGL` (version 1.59.5) reports the error that
`BGL_brandes_betweeness_centrality` not available for `.Call()` for package
`r Biocpkg("RBGL")`. Therefore the execution has been suppressed for now.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Accio I just wanted to make a note of this section that references RBGL 1.59.5 (it's now 1.79.0).

Copy link
Contributor

@jwokaty jwokaty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sonali8434 Thanks for this PR. I've made some small inline requests. The DESCRIPTION file should have VignetteBuilder: knitr and BiocStyle, knitr in Suggests. Because this is missing, R CMD build isn't generating the vignette.

I also want to confirm with @Accio if the new bibliography format is suitable as it presents the reference differently. For example

Current:

[Aittokallio and Schwikowski, 2006] Aittokallio and Schwikowski (2006) Graph-based methods for analysing networks in cell biology, Briefings in Bioinformatics, 7, 243-255.

Newer:

Aittokallio and Schwikowski (2006) Graph-based methods for analysing networks in cell biology. Briefings in Bioinformatics, 7, 243–255.

@sonali8434
Copy link
Author

@jwokaty Thanks for the review, and I'm sorry for not pushing my changes to the description file. Thanks for reminding me.

I have made the changes you asked for. Please review.

Copy link
Contributor

@jwokaty jwokaty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sonali8434 Thanks for the changes. This is ready for @Accio.

@Accio
Copy link
Owner

Accio commented Aug 22, 2023

Dear @sonali8434 and @jwokaty, I agree with the changes you proposed. Thank you for making them.

Do you mind adding yourself as contributors in the DESCRIPTION file? I am glad to merge the changes once the testing passes.

Have a good day and thanks again for the generous help, David.

@sonali8434
Copy link
Author

@Accio, I have completed the remaining changes. Please review the file and let me know if any more changes are required.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants