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 Chromband.Rnw to Chromband.Rmd #4

Closed
wants to merge 3 commits into from

Conversation

kemicky
Copy link

@kemicky kemicky commented Mar 24, 2023

@Bioconductor/sweave2rmd: please may you review Chromband sweave2rmd ?, thank you
ChromBand.pdf

ChromBand.html

@jwokaty
Copy link
Contributor

jwokaty commented Mar 24, 2023

Hi @kemicky, good job making the PR.

Your PR should include

  • addition of the new .Rmd file
  • removal of the .Rnw file
  • edits to the DESCRIPTION file

No other files should be added to or altered in this PR. Usually files like this get included by doing git add .. It's good to get in the habit of explicitly writing out the files you want to include so that you don't make unnecessary additions. Can you please take care of these extra files?

@kemicky
Copy link
Author

kemicky commented Mar 24, 2023

Hi @kemicky, good job making the PR.

Your PR should include

  • addition of the new .Rmd file
  • removal of the .Rnw file
  • edits to the DESCRIPTION file

No other files should be added to or altered in this PR. Usually files like this get included by doing git add .. It's good to get in the habit of explicitly writing out the files you want to include so that you don't make unnecessary additions. Can you please take care of these extra files?

Good day everyone, thank you for your review, I did the edits to the description file, removed the Chromband.Rnw file , added the .Rmd. file. and as instructed I will remove the extra files.
But please to clarify the new ChromBand.Rmd file , I created the table using the Chromband.xlsx file and as well added the image using the image in the figure files so if I remove them might alter the table and images, please any advice on that too. Thank you very much.

@jwokaty
Copy link
Contributor

jwokaty commented Mar 24, 2023

I looked at the existing files in the devel branch and no images exist there, so we should be able to generate them by compiling the .Rmd. Anything that we can regenerate, we don't commit. If they are not being regenerated, we will have to fix this during the review.

@kemicky
Copy link
Author

kemicky commented Mar 24, 2023

The Images and the table are included in the original .rnw file as figure and tb but after they were converted to .Rmd they were not formatted correctly, So I only just fixed it to look as the pdf as instructed in the contribute article.
May you please see, here is a screenshot of the original ChromBand.Rnw
image of the original ChromBand Rnw file

@jwokaty jwokaty changed the title File rmd Convert Chromband.Rnw to Chromband.Rmd Mar 25, 2023
Comment on lines +30 to +35
```{r,echo=FALSE, eval=FALSE}
if (!requireNamespace("BiocManager", quietly = TRUE))
install.packages("BiocManager")
BiocManager::install("Category")
```

Copy link
Contributor

@jwokaty jwokaty Mar 25, 2023

Choose a reason for hiding this comment

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

Remove lines 30-35. They are not in the existing PDF.

Comment on lines +36 to +50
::: Schunk
::: Sinput
\> library("Category")\
\> library("ALL")\
\> library("hgu95av2.db")\
\> library("annotate")\
\> library("genefilter")\
\> ##library("SNPchip")\
\> library("karyoploteR")\
\> library("geneplotter")\
\> library("limma")\
\> library("lattice")\
\> library("graph")\
:::
:::
Copy link
Contributor

@jwokaty jwokaty Mar 25, 2023

Choose a reason for hiding this comment

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

Convert lines 36-50 into a code block. The code for the figures depends on this code.

It should resemble the following

```{r setup}
library("Category")
library("ALL")
```

Also, later in the code you have another code block named as setup, which will error when you try to knit. This code block should be named setup like the original document. I recommend removing the other code block as it sets echo = TRUE for all the code blocks; however, that is the default setting.

Comment on lines +56 to +66
<figure id="fig:chr12ideogram">

::: center
```{r chr12ideogram, echo=FALSE, fig.align="center", out.width="100%", out.height="100%"}
knitr::include_graphics("figures/chr12ideogram.png")
```
:::

<figcaption>Ideogram for human chromosome 12. The p arm is on the left, the q arm is on the right, and the centromere is indicated by a notch. The shaded bands together represent 12q21. This band is composed of three sub-bands: 12q21.1, 12q21.2, and 12q21.3. The last of these is composed of sub-sub-bands 12q21.31, 12q21.32, and 12q21.33.</figcaption>

</figure>
Copy link
Contributor

@jwokaty jwokaty Mar 25, 2023

Choose a reason for hiding this comment

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

Replace 56-66 with the code block to generate the figure. Create a code block containing the code in https://github.com/Bioconductor/Category/pull/4/files#diff-fe345a9843884e8331a5988cdd3d46c9275d714c6e87261a3822aa3f63aba921L122-L136. Keep chr12ideogram as the name. To display the code properly, you'll have to provide options: fig.width, fig.height, fig.align, and set the text in line 64 as fig.cap. Set echo = FALSE to prevent display of the code.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, thank you very much for your wise guided instructions, I am on it and fixing them immediately . I am grateful.

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.

I've left comments to help guide you to fix the first figure in the .Rmd file. Part of the problem is that the code is just text. You need to convert code sections into code blocks so that they can be run/compiled. Code in vignettes gradually build up because vignettes explain how to use the functions in the package. If you miss part of the code, the rest of the code below it may not run properly.

You'll need to convert a lot of sections to code blocks. I recommend using the PDF and .Rnw file as guides. Please try these first steps and as I mentioned previously, remove other files. If you are able to get the first figure generating, I recommend trying the same approach with the others.

@kemicky kemicky closed this Mar 28, 2023
@kemicky kemicky deleted the file-Rmd branch March 28, 2023 18:00
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.

2 participants