Bioconductor / Biostrings

Efficient manipulation of biological strings
https://bioconductor.org/packages/Biostrings
54 stars 16 forks source link

Converting MultipleAlignments.Rnw to Rmarkdown #90

Closed BerylKanali closed 3 months ago

BerylKanali commented 1 year ago

@jwokaty Take a look at the changes PDF and HTML for comparison Issues I have:

    • In the description file when writing authors using this format
      Authors@R: c(
      person("Hervé", "Pagès", role=c("aut", "cre"),

      is it okay for the first name to be the intials as in the previous format.

  1. Graph position not as exact as PDF
  2. Size of graph(someone should not scroll to thee the whole graph)
  3. The dev.offgives a different output when I knit
null device 
          1 

is the correct output but after in the HTML it shows

## png 
##   2
jwokaty commented 1 year ago

Let's follow the previous format using initials for the DESCRIPTION.

Looking at https://github.com/Bioconductor/Biostrings/blob/c94e8fb082601cf3e3998df82cf1a9b39c72cb27/vignettes/MultipleAlignments.Rnw#L278-L283, I suspect that the original authors moved the figure to waste less space on the pages of the PDF. Let's allow the figures to follow their code naturally since the HTML document will not have the same problem.

Since the PDF isn't displaying as nicely as we'd like, I'd like to ask @hpages if we can change pdf to png instead? Or if there's a way to make the PDF display nicely, that suggestion would be helpful.

Lastly, I think we don't have to worry about the value of dev.off() as this is turning off the device and returning the value of that device I believe (we can check with ?dev.off). dev.off is always the "stub", which is null device 1, on the build system, which will generate the vignettes available on bioconductor.org. For users running this code, it will almost always show something else.

BerylKanali commented 1 year ago

@jwokaty This is ready for review apart from the graph issue talked about above. I did R CMD build and R CMD check and found no errors.

hpages commented 1 year ago

Can we use .pngs for the figures rather than .pdfs since they don't format well?

of course, thanks

BerylKanali commented 1 year ago

@jwokaty I made the requested changes. Updated HTML for comparison with PDF

hpages commented 1 year ago

@BerylKanali @jwokaty Where are we standing with this PR?

jwokaty commented 1 year ago

I can check that the issues are resolved then notify @hpages.

jwokaty commented 1 year ago

@hpages This is ready for your review.

ahl27 commented 3 months ago

@hpages pinging to follow up on conversation from the other PR--is this also something that we need to merge?

hpages commented 3 months ago

Just replaced Biostrings2classes.Rnw with Beryl's Biostrings2classes.Rmd (see commit a1fae35686d2fea09e896bd06a902c8ad1d5bbdf). Thanks for the reminder.